WARNING, this is a long shot and involves a few parts in drupal.
A bit of history is here :
https://www.drupal.org/node/2385069 -> discussing why remote urls get to locale.inc system trough hook_js_alter
https://www.drupal.org/node/1802394 -> showing how a contrib module can use drupal_add_js badly ( and still in stable )
So for drupal_add_js :
If no type is specified and the URL is external, it would be wise to force the type to external, otherwise modules relying on the type to alter javascript will bypass drupal_http_request method, and potentially allow access to forbidden resource on the network by bypassing any configured proxy.
This is a safety against badly coded module that could open a potential breach to the internal network.
On the wysiwyg/locale combo the issue is not a security hazard because the included JS is a fixed one, but I can definitely see some modules asking for JS urls at the backoffice level, skip the external part and then, therefore lets to potential security risk.
( example, filling http://192.168.1.1/reboot.asp would reboot your home router, in corporate it could get way worse ).
The patch provided makes sure that no external URLs are added without beeing tagged a external.
It doesn't touch the type if the user forces it tough, so only modules forgetting to set will change behavior.
It will also improve performance on sites that tends to use badly coded modules as the server will never query itself trough http anymore if not needed.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | drupal_add_js_sanitize_external-2697611-4.patch | 707 bytes | ademarco |
Comments
Comment #2
gboddin commentedComment #3
gboddin commentedBtw, I agreed it's about trust and environment again, so I let you guys decide what to do with it ;), I just think it can't hurt.
For the wysiwyg module they're explicitely setting the type to "file" even when it's external so this patch doesn't apply.
For them the issue is known and they should probably release a new version ... 2.2 is from 2012 and people are crying for it.
Comment #4
ademarco commentedRe-rolled taking into consideration that $data can be an array. I've also fixed a few Drupal coding standard issues.
Comment #6
ademarco commentedComment #7
ademarco commented