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 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 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 commented6: image-hook_requirements_insecure_derivatives-1935850-6.patch queued for re-testing.
Comment #22
smustgrave commentedThank you for creating this issue to improve Drupal.
We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.
Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!
Comment #23
smustgrave commentedSince there's been no follow up in 3 months going to close this one out. But if still a valid task we can always re-open
Thanks!