Denver, Colorado, USA
Account created on 21 October 2005, over 19 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States greggles Denver, Colorado, USA

This help message is not helpful. It's just copy paste from the project page, which people will already know.

Marking wont' fix.

🇺🇸United States greggles Denver, Colorado, USA

Thanks for reporting this issue and suggesting a workaround.

I assume it still affects the latest version of 8.x-1.x so updating the version to that. If it's been fixed since please leave a comment about that.

🇺🇸United States greggles Denver, Colorado, USA

Seems like a simple enough change. I turned the patch into an MR, but credit to mattjones86.

I confirmed that --preserve-perms is a valid option.

🇺🇸United States greggles Denver, Colorado, USA

greggles made their first commit to this issue’s fork.

🇺🇸United States greggles Denver, Colorado, USA

It seems ideal to fix #2992359: Use the symfony process component before working on any more binaries. Additional binaries should ideally be written using the symfony process compontent. Marking this as postponed on that.

🇺🇸United States greggles Denver, Colorado, USA

That sounds like not all of the files were present on the site.

Can you try replicating this issue on a brand new "test" site and see if the issue is present there?

🇺🇸United States greggles Denver, Colorado, USA

It seems ideal to fix #2992359: Use the symfony process component before working on any more binaries. Additional binaries should ideally be written using the symfony process compontent. Marking this as postponed on that.

🇺🇸United States greggles Denver, Colorado, USA

I read on the project page

libjpeg-turbo implements both the traditional libjpeg API as well as the less powerful but more straightforward TurboJPEG API.

(emphasis mine)

I think the advice from the earlier comment to point this library at the libjepg-turbo binary should work, so marking this as "fixed."

🇺🇸United States greggles Denver, Colorado, USA

I'm +1 to enabling phpcs in GitLab as the MR does. The code sniff identified many adjustments, so it might not be practical to fix them all in this issue. I've sometimes used the practice of merging the enabling the codesniffer with known problems and then fixing them slowly over time.

🇺🇸United States greggles Denver, Colorado, USA

I imagine by now that you have an answer to the question. Without trying it myself and relying on some outdated memories - As I understand it, the default 75% only affects a pipeline if you have a step that will resample the file before sending it to resmushit. I guess it does not affect a typical pipeline that just sends to resmushit.

As a support request I guess this is "fixed".

🇺🇸United States greggles Denver, Colorado, USA

Updating version to a supported branch. I think the "Needs work" status is still accurate.

🇺🇸United States greggles Denver, Colorado, USA

8.x-2.x branch is no longer supported, so moving to needs work for a porting.

Is this feature request still relevant for the 4.x branches? If so it seems the version should be updated. If not I guess this could be closed as outdated. Thanks!

🇺🇸United States greggles Denver, Colorado, USA

Does this issue affect the current 4.x branches?

🇺🇸United States greggles Denver, Colorado, USA

Re-titling this to state it in a way that I think could potentially work. I can definitely understand the desire for this. I'm not sure if this idea is worthwhile or not from the perspective of the maintainers of the module to add more code to handle this scenario.

🇺🇸United States greggles Denver, Colorado, USA

It looks like the 4.x branches do not have drush integration, so moving this feature request to that version.

🇺🇸United States greggles Denver, Colorado, USA

I see some tests in the current "modern Drupal" codebase, so I think "Fixed" might be the best status here.

If there are other kinds of tests you had in mind, could you elaborate, reopen, and reassign the version?

🇺🇸United States greggles Denver, Colorado, USA

Thanks for the explanation in #3. Since 7.x-2.x is no longer supported I think this is the best status.

🇺🇸United States greggles Denver, Colorado, USA

The 8.x-2.x branch is no longer supported and the remaining 4.x branches have support for modern Drupal, so I guess closing this as outdated is the most accurate option.

🇺🇸United States greggles Denver, Colorado, USA

Closing after a while without a response to the question in #2.

If this is still relevant for currently supported versions of the module, please reopen it.

🇺🇸United States greggles Denver, Colorado, USA

Agreed with this proposal and with the specific task to finish before that.

🇺🇸United States greggles Denver, Colorado, USA

Thanks for the report and research on this issue.

Is this still relevant with the latest releases of the module?

🇺🇸United States greggles Denver, Colorado, USA

Thanks for the issue report.

Is this still happening with the latest releases?

🇺🇸United States greggles Denver, Colorado, USA

greggles created an issue.

🇺🇸United States greggles Denver, Colorado, USA

Yep, we met.

🇺🇸United States greggles Denver, Colorado, USA
🇺🇸United States greggles Denver, Colorado, USA

greggles created an issue.

🇺🇸United States greggles Denver, Colorado, USA

Thanks for the reminder. That's now done.

🇺🇸United States greggles Denver, Colorado, USA

I think this needs someone from the DA who manage code for the site. They can adjust this in the views UI and export it to the feature.

🇺🇸United States greggles Denver, Colorado, USA

An overlap seems fine, but I'd also be fine if we just did as a "big bang migration" it if that helps get it done faster.

🇺🇸United States greggles Denver, Colorado, USA

removed outdated issues, updated from http to https, updated a variety of areas for more common current issues

🇺🇸United States greggles Denver, Colorado, USA

Thanks for the research in #31/32 and for the explanation in #33.

+1 to swapping out that error page to a service that is visible without having to log in. Mastodon seems good. If someone can help advise on how to embed a mastodon feed that might help move this along. Absent that I guess just linking to the Mastodon feed is better than linking to an inaccessible service.

For what its worth, the Drupal Security Advisories are now broadcast in 3 places: x.com,
bluesky, and the drupal.community mastodon instance. I don't have data about which service is most valuable, but you can compare engagement (RT, likes) to get a rough sense of which is most valuable.

I recently automated auto-posting of the security RSS feeds to bsky and mastodon, which has no cost other than the time to configure it (which was relatively low). I believe we use a commercial service for Twitter due to the cost of their API (I don't administer that servce, so not sure of the details).

🇺🇸United States greggles Denver, Colorado, USA

It took me a second to understand the idea, but I think this will be a good improvement. Thanks for your effort on it.

🇺🇸United States greggles Denver, Colorado, USA
🇺🇸United States greggles Denver, Colorado, USA
🇺🇸United States greggles Denver, Colorado, USA

Ping me if you need access to this.

🇺🇸United States greggles Denver, Colorado, USA

The title is SSRF (server side request forgery) which I understand to mean a request from the server to other machines on the server's network as described in portswigger and owasp.

The description includes:

The vulnerability allows one Drupal user with administrative permissions to create a page which can force another Drupal user to make a call to a server under the control of an attacker.

and

The browser will make a call to the malicious server

That is not a server-side request.

I can see how this would let an admin gain data about the site visitors, which is not ideal, but on most sites an admin who can enter translations can probably already make other changes to a site that would also gain information about a visitor.

I'm surprised if this is the only place where a user with only the ability to translate could insert an image. I think other strings are similarly not filtered.

🇺🇸United States greggles Denver, Colorado, USA

I'm not sure if project maintainers can do that, but I've now done it.

Thanks to you both for the collaboration and coordination on ensuring active and accurate maintainers.

🇺🇸United States greggles Denver, Colorado, USA

Adding what I believe is the audit ignore syntax to the issue summary to help folks as a workaround until this gets packaged.

🇺🇸United States greggles Denver, Colorado, USA
🇺🇸United States greggles Denver, Colorado, USA
🇺🇸United States greggles Denver, Colorado, USA
🇺🇸United States greggles Denver, Colorado, USA

I noticed there's a lot of "warning" text near the top of project page that could probably be swapped for more inviting formatting. The webform page has a few examples that seem to leverage d.o styles in different ways without being as offputting/warning.

🇺🇸United States greggles Denver, Colorado, USA

I found this issue while looking for a duplicate before creating one.

I agree with the issue summary.

Yes, I would say it's important for there to be a way for a site developer, with access to settings.php, to disable the ability to update modules or install new modules from remote locations. It's either very similar to allow_authorize_operations and should be documented/placed in a similar location, or should rely on allow_authorize_operations directly.

Updating the IS to remove some ambiguity about whether this is a good idea.

🇺🇸United States greggles Denver, Colorado, USA

removing duplicate Gerhard

🇺🇸United States greggles Denver, Colorado, USA

@klemendev can you review those as well and add your comments on them if they seem RTBC to you?

🇺🇸United States greggles Denver, Colorado, USA
🇺🇸United States greggles Denver, Colorado, USA

Explain a bit more about what CVEs are and how to help write them.

🇺🇸United States greggles Denver, Colorado, USA

explaining to use a predefined risk score

🇺🇸United States greggles Denver, Colorado, USA
🇺🇸United States greggles Denver, Colorado, USA
🇺🇸United States greggles Denver, Colorado, USA

Thanks, everyone. This is now done.

🇺🇸United States greggles Denver, Colorado, USA

Can you update the IS with the permissions for the user that makes this a bug? And maybe what happens if that user goes to user/1

It seems like an uncommon configuration to be able to set privileged fields like author of a node and not see usernames, but the behavior should be consistent if that configuration does happen.

🇺🇸United States greggles Denver, Colorado, USA

Ah, that makes sense. +1 to that as a base level.

Personally I've generally asked applicants to go through the queue and post patches to the things they are likely to fix first, to provide feedback on proposed feature issues of whether an idea fits with the philosophy of the module or not, to do reviews of patches other people have provided, to create a "next release" plan issue and highlight what they would commit in that next release. Those create more material for the current maintainer to at least skim over before granting access. They would also get the person past being "new" without using that specifically as the barrier.

🇺🇸United States greggles Denver, Colorado, USA

There are docs pages already, we could definitely add some suggested items to the template. I'd be more +1 to the idea of using "git verified" as a hurdle if that process were more efficient and transparent to end users (e.g. a quiz, being able to apply with significant MRs to core/contrib).

🇺🇸United States greggles Denver, Colorado, USA

Re-titling. It seems important to focus on the goal (safer additions) than a potential solution (harder additions). It's possible a solution will be that it is somewhat harder to be added as co-maintainer, but just making it harder alone doesn't make it safer.

🇺🇸United States greggles Denver, Colorado, USA

We now have a field for composer version strings. These are not always completed, but maybe they are good enough?

The script in 📌 Create CVEs for contributed projects in 2024 Active uses that field to populate the version string on CVEs so I think this could now be closed as fixed.

I recognize "things are changing" with the Gitlab migration so we might need to revisit this in the future.

🇺🇸United States greggles Denver, Colorado, USA
🇺🇸United States greggles Denver, Colorado, USA

Analysis to date in the private issue is that this cannot be exploited, but we should ensure this code is resilient and won't be weakened in the future.

🇺🇸United States greggles Denver, Colorado, USA

Maybe this should be won't fix after Tour is deprecated? https://www.drupal.org/node/3421972

🇺🇸United States greggles Denver, Colorado, USA

@emjayess that error message comes from core's system.install.

Here's an issue that I think is relevant to your goal of making it more appropriate for nginx users 🐛 Remove error “Public files directory Not fully protected” in non apache servers Needs review

🇺🇸United States greggles Denver, Colorado, USA

I discarded the email already in mailman moderation queue, but if/when it crops up again I will preserve the full email so we can do more investigation.

🇺🇸United States greggles Denver, Colorado, USA

@emjayess thanks for the followup. Can you post a screenshot of the message in the status report?

🇺🇸United States greggles Denver, Colorado, USA
🇺🇸United States greggles Denver, Colorado, USA

greggles created an issue.

🇺🇸United States greggles Denver, Colorado, USA

I think this should be "postponed" until after 🌱 Drop support of 8.x-1.x version Active is completed and after a fair number of folks are on 2.1.0. I won't flip the status myself in case there's another way to run it that seems good to you.

🇺🇸United States greggles Denver, Colorado, USA

This seems like a very thorough plan.

There are no bugs reported against the 2.0.x branch, so that's a good sign.

Currently, the usage of the module is:

Week	7.x-1.x	8.x-1.x	2.0.x	Total
December 28, 2024	362	673	789	1,824

As long as the upgrade path works well I think this is a great idea.

🇺🇸United States greggles Denver, Colorado, USA

adding bramdriesen

🇺🇸United States greggles Denver, Colorado, USA

I updated the advisory for 2025-005 to point to that CVE.

2024-001 is CVE-2024-11941 and now points to it.

@cmlara are you willing to research the CWE and CAPEC for these?

🇺🇸United States greggles Denver, Colorado, USA

Don't think I'll need this for a bit. Might reopen it if I do. Thanks.

🇺🇸United States greggles Denver, Colorado, USA

Oh, and here are the links:

https://www.cve.org/CVERecord?id=CVE-2024-13312
https://www.cve.org/CVERecord?id=CVE-2024-13311
https://www.cve.org/CVERecord?id=CVE-2024-13310
https://www.cve.org/CVERecord?id=CVE-2024-13309
https://www.cve.org/CVERecord?id=CVE-2024-13308
https://www.cve.org/CVERecord?id=CVE-2024-13305
https://www.cve.org/CVERecord?id=CVE-2024-13304
https://www.cve.org/CVERecord?id=CVE-2024-13303
https://www.cve.org/CVERecord?id=CVE-2024-13302
https://www.cve.org/CVERecord?id=CVE-2024-13301
https://www.cve.org/CVERecord?id=CVE-2024-13300
https://www.cve.org/CVERecord?id=CVE-2024-13299
https://www.cve.org/CVERecord?id=CVE-2024-13298
https://www.cve.org/CVERecord?id=CVE-2024-13297
https://www.cve.org/CVERecord?id=CVE-2024-13296
https://www.cve.org/CVERecord?id=CVE-2024-13295
https://www.cve.org/CVERecord?id=CVE-2024-13294
https://www.cve.org/CVERecord?id=CVE-2024-13293
https://www.cve.org/CVERecord?id=CVE-2024-13292
https://www.cve.org/CVERecord?id=CVE-2024-13291
https://www.cve.org/CVERecord?id=CVE-2024-13290
https://www.cve.org/CVERecord?id=CVE-2024-13289
https://www.cve.org/CVERecord?id=CVE-2024-13288
https://www.cve.org/CVERecord?id=CVE-2024-13287
https://www.cve.org/CVERecord?id=CVE-2024-13286
https://www.cve.org/CVERecord?id=CVE-2024-13285
https://www.cve.org/CVERecord?id=CVE-2024-13284
https://www.cve.org/CVERecord?id=CVE-2024-13283
https://www.cve.org/CVERecord?id=CVE-2024-13282
https://www.cve.org/CVERecord?id=CVE-2024-13281
https://www.cve.org/CVERecord?id=CVE-2024-13280
https://www.cve.org/CVERecord?id=CVE-2024-13279
https://www.cve.org/CVERecord?id=CVE-2024-13278
https://www.cve.org/CVERecord?id=CVE-2024-13277
https://www.cve.org/CVERecord?id=CVE-2024-13276
https://www.cve.org/CVERecord?id=CVE-2024-13275
https://www.cve.org/CVERecord?id=CVE-2024-13274
https://www.cve.org/CVERecord?id=CVE-2024-13273
https://www.cve.org/CVERecord?id=CVE-2024-13272
https://www.cve.org/CVERecord?id=CVE-2024-13271
https://www.cve.org/CVERecord?id=CVE-2024-13270
https://www.cve.org/CVERecord?id=CVE-2024-13269
https://www.cve.org/CVERecord?id=CVE-2024-13268
https://www.cve.org/CVERecord?id=CVE-2024-13267
https://www.cve.org/CVERecord?id=CVE-2024-13266
https://www.cve.org/CVERecord?id=CVE-2024-13265
https://www.cve.org/CVERecord?id=CVE-2024-13264
https://www.cve.org/CVERecord?id=CVE-2024-13263
https://www.cve.org/CVERecord?id=CVE-2024-13262
https://www.cve.org/CVERecord?id=CVE-2024-13261
https://www.cve.org/CVERecord?id=CVE-2024-13260
https://www.cve.org/CVERecord?id=CVE-2024-13259
https://www.cve.org/CVERecord?id=CVE-2024-13258
https://www.cve.org/CVERecord?id=CVE-2024-13257
https://www.cve.org/CVERecord?id=CVE-2024-13256
https://www.cve.org/CVERecord?id=CVE-2024-13255
https://www.cve.org/CVERecord?id=CVE-2024-13254
https://www.cve.org/CVERecord?id=CVE-2024-13253
https://www.cve.org/CVERecord?id=CVE-2024-13252
https://www.cve.org/CVERecord?id=CVE-2024-13251
https://www.cve.org/CVERecord?id=CVE-2024-13250
https://www.cve.org/CVERecord?id=CVE-2024-13249
https://www.cve.org/CVERecord?id=CVE-2024-13248
https://www.cve.org/CVERecord?id=CVE-2024-13247
https://www.cve.org/CVERecord?id=CVE-2024-13246
https://www.cve.org/CVERecord?id=CVE-2024-13245
https://www.cve.org/CVERecord?id=CVE-2024-13244
https://www.cve.org/CVERecord?id=CVE-2024-13243
https://www.cve.org/CVERecord?id=CVE-2024-13242
https://www.cve.org/CVERecord?id=CVE-2024-13241
https://www.cve.org/CVERecord?id=CVE-2024-13240
https://www.cve.org/CVERecord?id=CVE-2024-13239
https://www.cve.org/CVERecord?id=CVE-2024-13238
https://www.cve.org/CVERecord?id=CVE-2024-13237

🇺🇸United States greggles Denver, Colorado, USA

OK. Done! Thanks to everyone for providing your analysis and feedback.

Some deficiencies I noticed in the json generated by this script that may not be worth fixing:
Something is corrupted in special characters like á from https://www.drupal.org/sa-contrib-2024-020
The 7.x versions should be described as "custom" instead of "semver"

I realized I could create the unsupported advisories without CWE and CAPEC by removing those fields from vulnogram (I had read they were optional earlier, but thought maybe vulnogram was going to require it). So I did that.

There are no doubt a small number of instances of inaccurate data in these CVEs compared to their advisories, especially as it relates to version strings and 7.x values. There may also be more appropriate or additional CWE and CAPEC values that we should add.

I'm going to mark this issue as fixed since the initial effort is done.

If anyone notices any inaccurate data in these please open new issues in this queue and I should be able to address them quickly.

🇺🇸United States greggles Denver, Colorado, USA

Adding my +1 to the perspective in #26.

Given that Drupal CMS smooths over a lot of details from site builders it seems important to make reasonable decisions for them that match their preferences/risk profile.

🇺🇸United States greggles Denver, Colorado, USA

@kentr I think some bulk updates are coming to address some issue metadata, but agreed this issue should stay focused on getting the MR into D11 and not worry about D7. Removing the "Needs backport to D7" issue tag.

🇺🇸United States greggles Denver, Colorado, USA

No advisory is needed from Drupal as we're not the vendor. There are tons of 3rd party software that get integrated with Drupal and have vulnerabilities regularly that we don't issue advisories for. Some past advisories reinforcing this practice are https://www.drupal.org/psa-2024-06-26

🇺🇸United States greggles Denver, Colorado, USA

I don't think there is a way to mark older releases as unsupported. Branches can be marked as unsupported, but not releases.

🇺🇸United States greggles Denver, Colorado, USA
🇺🇸United States greggles Denver, Colorado, USA

adding explanation for handling an advisory being published after the fix is already public

🇺🇸United States greggles Denver, Colorado, USA

TIL - thanks @solideogloria! It would be ideal to know the number of Drupal sites among those, but I recognize that may not be possible.

🇺🇸United States greggles Denver, Colorado, USA

Updating title to clarify this affects drupal.org and not the Drupal software more generally.

🇺🇸United States greggles Denver, Colorado, USA

Maybe the best short-term answer here is not to show anything.

In the long run it seems great if we could do the recursive calculating you mention, catch, and then have icons/text to explain "all modules are covered with stable releases" or "It's a mix of statuses and stable releases, get details."

🇺🇸United States greggles Denver, Colorado, USA

It seems important to show the coverage before the modules are installed on a site. The status is pretty prominent on the project page to help people make informed choices.

🇺🇸United States greggles Denver, Colorado, USA

Thanks for the thoughts in #10.

I'm inclined to take small steps here and see how the process of updating it this time goes before we make a big change to the scope. Most of the other organization scopes I reviewed have rather brief and broad scope statements, so I'm hesitant to enshrine too much of the project's practices into the scope.

I guess a good system would be to revisit this periodically (every year or so?) to ensure it matches the current practices.

🇺🇸United States greggles Denver, Colorado, USA

The issue summary says:

Nginx is one of the most popular web servers on which Drupal is run.

Does anyone have data about the level of popularity?

🇺🇸United States greggles Denver, Colorado, USA

I think comment #10 got flagged by the d.o auto moderation system. I've published it and confirmed nitinpatil's account so that won't happen again. Welcome nitinpatil and thanks for your comment on this issue!

🇺🇸United States greggles Denver, Colorado, USA

The concept of the check for CSP header on the status page feels a like a pretty big additional functionality for core. It seems like something security_review could handle, so I filed Check for CSP on private and public SVG files Active to track it there.

🇺🇸United States greggles Denver, Colorado, USA

greggles created an issue.

🇺🇸United States greggles Denver, Colorado, USA

Some other questions to consider - I've added numbers to them to facilitate discussion.

1 Could the .htaccess change interfere with the use of typical
theme-provided SVGs?

Great question. I don't think it's very common for svgs to use these features (I've only seen it once on a non-Drupal mapping site) but it made me realize the change could be inside of the htaccess for the files directory and therefore only apply to a subset of files. That feels like it reduces the risk this change will negatively impact while still retaining most of the value in protecting sites from malicious files. I asked mr.baileys and he agreed on that idea, but we could certainly discuss it more.

2 Do SVGs pose risks for users outside of an http request, eg when downloading them and viewing them? This risk of course exists with other file types, but less so for jpg/png images.

I think they do pose a risk, but solving it feels like a separate problem to me. I filed Filter SVGs before uploading by default Postponed and linked it to this as related for followup that could address this risk. The situation today is that site admins are enabling SVG uploads on their site often and there is no protection for a variety of risks that creates. This issue attempts to solve one of the risks (XSS), but is not a complete protection against all risks.

3 Are typical XML-issues such as XXE, XSLT a possible issue, if so, in what contexts?

As far as I understand those risks they are not relevant on a typical Drupal site and would only be relevant on sites that parse the XML of the SVG file. These changes don't affect that for better/worse.

* Are we sure all possible threats from SVGs in img/object/background contexts are tackled? Which specs apply?

I'm not sure this mitigates all possible threats (several additional threats are mentioned in this issue already). I'm sure the change in this issue is progress in securing sites against cross site scripting, but not a solution to all risks of SVG files.

🇺🇸United States greggles Denver, Colorado, USA

It...appears I failed to include the link to the gist in comment #18.

Here is the script to generate cve jsons https://gist.github.com/greggles/e3c525af1790c05b1ff882eee826f0fd

As I said there, php advisory-to-cvejson.php and it will create the jsons. Note that the URLs on line 202/203 need to be manually edited to create all of them, i.e. comment out line 203 to generate the most recent advisories.

Production build 0.71.5 2024