Multiple vulnerabilities in 7.x-1.x and 7.x-2.x

This issue was originally reported to the Drupal security team by mparker17

Original report

Advisory:

This module does not define it's own permissions: it uses "administer users" instead.

***

This module has a XSS vulnerability.

You can see this vulnerability by:

1. Enabling the module and installing the required library.
2. As a user with the "administer users" permission go to admin/config/system/jreject, click "Enable jReject" and, under "Modal paragraph 1", enter the text Your browser is out of date, and may not be compatible with our website. A list of the most popular web browsers can be found below. <script>alert();</script>
3. Visit the site in any browser enabled at admin/config/system/jreject/browsers (e.g.: a VM of IE 7 from the official http://modern.ie/ site).

Note this issue is not technically covered by the Security Advisory Policy because it requires advanced permissions ("administer users") to save the config settings; but that's only because this module does not define it's own permissions (see first vulnerability note).

***

This module has a XSS vulnerability caused by running user-entered strings through the t() function.

From the t() function documentation:

It is especially important never to call t($user_text);, where $user_text is some text that a user entered - doing that can lead to cross-site scripting and other security problems.

I tried to come up with a test case for this, but Drupal's "Translate interface" UI rejects HTML in the translated string, and the "Import translation" UI skips any imported strings with HTML.

Unknown if this issue is covered by the Security Advisory Policy.

***

This module has a XSS vulnerability, as it allows any form element value whose name begins with "jreject_" to be written to database submitted to any of it's administration screens (admin/config/system/jreject, admin/config/system/jreject/options, admin/config/system/jreject/browsers, admin/config/system/jreject/alternatives), which is then output into JS in jreject_init().

I thought I'd be able to use Firebug manipulate the page DOM to add an element such as: <input name="jreject_foo" value="alert();" type="hidden">. Firebug tells me this value is submitted but Xdebug tells me it never makes into $form_state['values'] so maybe Drupal's Form API is catching this? Either way, jreject.admin.inc::jreject_prepare_settings() puts everything in $form_state['values'] into a $config value which is variable_save()d to the database in each _submit() function in jreject.admin.inc.

Note this issue is not technically covered by the Security Advisory Policy because it requires advanced permissions ("administer users") to save the config settings; but that's only because this module does not define it's own permissions (see first vulnerability note).

***

I'd be happy to write patches to fix some or all of these issues for both 7.x-1.x and 7.x-2.x.

***

Note that the 7.x-2.x branch is not covered by the Security Advisory Policy because it is in beta; however, it is vulnerable from all these things as well. AFAIK the only difference between them is that 7.x-2.x uses the Libraries API; otherwise the code is identical.

Original comments

#3 mparker17 commented March 12, 2015 at 11:02pm
Status	File
new	
REVIEW 2.56 KB152801-3.patch
Here's a patch for the advisory which adds a permission for this module.

Will work on the other issues shortly.
Comment #4 mparker17 commented March 12, 2015 at 11:30pm
Status:	Active	» Patch (code needs review)
Status	File
new	
REVIEW 3.56 KB152801-4.patch
new	
REVIEW 1.21 KBinterdiff.txt

Show1 file was hidden/shown/deleted
Here's a patch that fixes the first XSS vulnerability and mitgates the second XSS vulnerability.
Comment #5 mparker17 commented March 13, 2015 at 2:10am
Status	File
new	
REVIEW 4.15 KB152801-5-7x2x-do-not-test.patch
Here is also a straight re-roll of the patch in #4 for the 7.x-2.x version.
Comment #6 Pere Orga commented March 13, 2015 at 8:16am
Component:		» Code
Access:		» elevins, entendu
Thank you for reporting these issues to the Drupal Security Team.

Adding maintainers.

Maintainers: Please do not commit any code until requested to do so. Can you confirm these issues and review the provided patches?
Comment #7 Pere Orga commented March 21, 2015 at 7:55pm
Status:	Patch (code needs review)	» Needs maintainer response
We need an answer from the maintainers
Comment #8 Pere Orga commented April 11, 2015 at 6:44pm
I have contacted the maintainers via the contact form
Comment #9 Pere Orga commented April 27, 2015 at 8:21pm
Status:	Needs maintainer response	» Needs public issue created
We did not have an update on this.

However, as far as I can see these vulnerabilities may be fixed publicly as per http://drupal.org/security-advisory-policy because it requires the attacker to have an advanced permission that already makes the site compromised (the permission "Administer users" is marked as restricted access for trusted roles only). Even if the module does not set its own permissions, an user that has "Administer users" can create new administrators and is already able to inject JavaScript code on his own.

@mparker17 are you able to inject malicious code using an unrestricted permission? In that case, the module should be made unsupported.

Otherwise, please file a critical bug report against https://www.drupal.org/node/add/project-issue/jreject?tags=Security%20im... in the public issue queue and post the link here.

Regards