Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.In light of http://drupal.org/drupal-7.21-release-notes and Drupal 7.20 before that, I think this would be a good idea. We expect some sites to be using this variable for a little while longer, but it's still not fully secure and they shouldn't forget about it and leave it on forever.
Could maybe go in the Image Allow Insecure Derivatives module instead, but I think it makes sense in core.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | image-hook_requirements_insecure_derivatives-1935850-6.patch | 902 bytes | rootwork |
| #4 | image-hook_requirements_insecure_derivatives-1935850-4.patch | 906 bytes | bkonetzny |
Comments
Comment #1
RobLoachNot sure this check belongs in Drupal core. If anything, Drupal should be telling you to keep it on, since it's a security fix. Drupal core should not babysit broken code. The Image Allow Insecure Derivatives module is a good workaround in the mean time, at least until the broken code in contrib catches up.
Having a hook_requirements() check in the module itself is a great idea though. Put together an issue for that: #1936552: Add a hook_requirements().
Comment #2
David_Rothstein CreditAttribution: David_Rothstein commentedWhat I meant specifically is that if the variable is TRUE, there should be an error message on the requirements page telling you that your site is in an insecure state.
Comment #3
RobLoachAh, thanks for the clarification! Not sure we should be warning about this in Drupal core. The only way to enable it is by editing settings.php, or by enabling the module. If they did either one of those, they already know it's enabled and what they're getting themselves into. So having a warning just feels like needless spam at that point. Your call!
At the very least, the module definitely seems like an appropriate place for this. I'll stick that in right now. Thanks David!
Comment #4
bkonetzny CreditAttribution: bkonetzny commentedAs this is an security issue (DoS), we should warn about this in the status report. We have less important information we show up there (upload progress extension), so this one should be included.
Attached patch for D8. Not sure about the link to d.o, as this setting is only mentioned in the release notes of D7.20. Any other suggestions?
Comment #6
rootworkRemoved whitespace and added newline so that it would apply.
Comment #7
bkonetzny CreditAttribution: bkonetzny commented6: image-hook_requirements_insecure_derivatives-1935850-6.patch queued for re-testing.