Opened 10 years ago

Closed 10 years ago

Last modified 9 years ago

#10773 closed defect (fixed)

Multiple DOM-Based XSS in Dojo Toolkit SDK

Reported by: bix Owned by: bill
Priority: blocker Milestone: 1.4.2
Component: General Version: 1.4.0
Keywords: DOM XSS Cc:
Blocked By: Blocking:

Description (last modified by bill)


Multiple DOM-Based XSS in Dojo Toolkit SDK Adam Bixby - Gotham Digital Science

Affected Software: Dojo Toolkit SDK <= Build 1.4.1rc1

Browser used for testing: IE8 (8.0.7600.16385)

Severity: High


  1. Summary

The Dojo Toolkit is an open source modular JavaScript? library/toolkit designed to ease the rapid development of cross platform, JavaScript/Ajax? based applications and web sites. Multiple instances of DOM-based Cross Site Scripting (XSS) vulnerabilities were found in the _testCommon.js and runner.html files within the SDK. The XSS vulnerabilities appear to affect all websites that deploy any of the affected SDK files. These files are designed for testing, however a Google search identified numerous sites which have deployed these files along with the core framework components.

More information on DOM-based XSS can be found at http://www.owasp.org/index.php/DOM_Based_XSS.


  1. Technical Details

File: dojo-release-1.4.1-src\dojo-release-1.4.1-src\dijit\tests\_testCommon.js

1) Data enters via "theme" URL parameter through the window.location.href property.

Line 25:

var str = window.location.href.substr(window.location.href.indexOf("?")+1).split(/#/);

..snip..

2) The "theme" variable with user-controllable input is then passed into "themeCss" and "themeCssRtl" which is then passed to document.write(). Writing the un-validated data to HTML creates the XSS exposure.

Line 54:

  ..snip..
var themeCss = d.moduleUrl("dijit.themes",theme+"/"+theme+".css");
var themeCssRtl = d.moduleUrl("dijit.themes",theme+"/"+theme+"_rtl.css");
document.write('<link rel="stylesheet" type="text/css" href="'+themeCss+'">'); document.write('<link rel="stylesheet" type="text/css" href="'+themeCssRtl+'">');

File: dojo-release-1.4.1-src\dojo-release-1.4.1-src\util\doh\runner.html

1) Data enters via "dojoUrl" or "testUrl" URL parameters through the window.location.search property.

Line 40:

var qstr = window.location.search.substr(1);
  ..snip..

2) The "dojoUrl" and "testUrl" variables with user-controllable input are passed to document.write(). Writing the un-validated data to HTML creates the XSS exposure.

Line 64:

document.write("<scr"+"ipt type='text/javascript' djConfig='isDebug: true' src='"+dojoUrl+"'></scr"+"ipt>");
  ..snip..
document.write("<scr"+"ipt type='text/javascript' src='"+testUrl+".js'></scr"+"ipt>");

  1. Proof-of-Concept Exploit

This vulnerability can be exploited against websites that have deployed any of the 145 SDK files which reference _testCommon.js.

Reproduction Request: http://download.dojotoolkit.org/release-1.4.1/dojo-release-1.4.1/dijit/tests/form/test_Button.html?theme="/><script>alert(/xss/)</script>

(Note: test_Button.html is one of the SDK files that includes the _testCommon.js file)


This vulnerability can be exploited against any website that has deployed the runner.html file.

Reproduction Request: http://download.dojotoolkit.org/release-1.4.1/dojo-release-1.4.1/util/doh/runner.html?dojoUrl='/>foo</script><'"<script>alert(/xss/)</script>


  1. Recommendation

A JavaScript encoding function should be wrapped around the user-controllable variables to ensure that malicious data is properly encoded before rendering in the browser.

Attachments (1)

Ron Webb.jpg (167.8 KB) - added by Slavon8 4 years ago.
http://kdkraftupwnkitchen.tumblr.com/

Download all attachments as: .zip

Change History (23)

comment:1 Changed 10 years ago by James Burke

Description: modified (diff)
Milestone: tbd1.4.2

Thank you for the report. While we do not have a formal security policy set up, I believe normal security practice is to try to contact a team member in a private channel and allow us to work on the issue before making it public. Or to open the bug but not mention specifics until one of us can contact you privately.

The two sources of the vulnerabilities, testCommon.js and runner.html are test files. It is not expected that those files are deployed to production servers, although I can see how it might happen, and it is best to avoid the problems if we can, so we should fix the issues. I have marked this for the 1.4.2 release.

comment:2 Changed 10 years ago by bix

I did not see any area where security issues should be released, that's why I made the decision to release it here. I would recommend putting some verbiage in your FAQ on who to message when the security community has vulnerabilities to submit.

I did some google searching and found a bunch of sites that had these files in their webroot.

I have no problem deleting the details here until you've released a patch and then we can make it public.

Let me know what you think.

comment:3 Changed 10 years ago by bill

Description: modified (diff)
Owner: changed from anonymous to bill
Status: newassigned

I'll check in a fix. Not sure which "encoding function" you are talking about but AFAICT we can just filter out special characters as they shouldn't be occurring in the strings, using replace(). (I did so and the tests all still pass.)

comment:4 Changed 10 years ago by bill

Resolution: fixed
Status: assignedclosed

OK, fixed in [21405], for trunk/ and 1.4/. James, feel free to change if you see a better solution.

comment:5 Changed 9 years ago by bix

As an FYI, the recommended approach is to utilize a method such as escape() or encodeURIComponent() instead of performing character replacement.

comment:6 Changed 9 years ago by James Burke

bill: thank you very much for doing the fix.

bix: using encodeURIComponent does not seem to work at least for the the doh runner case? That case allows passing in a dojo URL, which seems like it will need un-encoded values. If you see a weakness with the fix, please let us know.

In a related thought, I am tempted to change the build default to not automatically copy tests, to hopefully reduce this type of problem in the future. While it will be a different behavior, I think it is safer, and easily fixed by someone wanting the old behavior via passing copyTests=true to the build command.

comment:7 Changed 9 years ago by bix

I understand that encoding isn't always possible. For those instances where you can't, I would recommend blacklisting the following potentially malicious characters if at all possible.

< > ! - % ; ) ( & + {} " '

Not copying tests files would definitely reduce the attack surface.

comment:8 Changed 9 years ago by bill

Thanks for the reminder about encodeURIComponent(). I think for DOH runner we'd need to use encodeURI() rather than encodeURIComponent(), to let the "/" characters through.

Like I said above, I don't think any of these values would legitimately contain special characters, because these values are used to construct file paths, and also because a theme name with (for example) a ">" character would terribly break CSS selectors. So, one could argue that it's odd to call encodeUri...() on a value that should never need encoding. But it could be argued either way, it's a philosophical thing.

comment:9 Changed 9 years ago by James Burke

(In [21413]) Refs #10773, making copyTests false by default

comment:10 Changed 9 years ago by James Burke

(In [21416]) Refs #10773, make sure remote urls cannot be used in the page, for 1.4 branch

comment:11 Changed 9 years ago by James Burke

I made copyTests=false by default, then also made the doh/runner.html checks more stringent to avoid : and urls that start with to avoid including script tags from other domains.

Give a holler if anything else should be done.

comment:12 Changed 9 years ago by bix

I was wondering if you had a release date for when these fixes will go live? I normally wait the standard 30 days or for a patch to be released before I post advisories to security lists. Please advise.

comment:13 Changed 9 years ago by James Burke

Looks like 1.4.2 will go out in about two weeks.

comment:14 in reply to:  13 Changed 9 years ago by bix

Replying to jburke:

Looks like 1.4.2 will go out in about two weeks.

Thanks jburke,

I'll definitely wait till then to release the advisory.

comment:15 Changed 9 years ago by James Burke

(In [21466]) Refs #10773 \!strict

comment:16 Changed 9 years ago by James Burke

(In [21467]) Refs #10773 \!strict

comment:17 Changed 9 years ago by James Burke

(In [21468]) Refs #10773 \!strict

comment:18 Changed 9 years ago by James Burke

(In [21479]) Refs #10773 \!strict

comment:19 Changed 9 years ago by James Burke

(In [21480]) Refs #10773 \!strict

comment:20 Changed 9 years ago by James Burke

bix: the refreshed builds are out, and this issue is part of a larger security advisory we just put out: http://dojotoolkit.org/blog/post/dylan/2010/03/dojo-security-advisory/

Thanks for reporting the issues. Note in the post above we have a new security mailing list if you need to discuss security matters in the future.

comment:21 Changed 9 years ago by bix

That's excellent jburke. I'll be releasing this as an advisory on Bugtraq as well.

comment:22 Changed 9 years ago by Douglas Hays

(In [23237]) Add check in _testCommon.js for parameter keys w/o values which throws an exception on IE7. Refs #10773.

Note: See TracTickets for help on using tickets.