yui / yui3

A library for building richly interactive web applications.
http://yuilibrary.com/
Other
4.13k stars 1.29k forks source link

potential security issue with the documentation example of JSONP #1177

Closed dmitris closed 11 years ago

dmitris commented 11 years ago

I was at the OWAP conference in Hamburg in August (http://appsec.eu), and Stefano di Paola, one of the security researches and Javascript experts, showed me a potential vulnerability in the recommended way of using JSONP in YUI, documented in http://yuilibrary.com/yui/docs/jsonp/.

The section "Customizing the JSONP URL" http://yuilibrary.com/yui/docs/jsonp/#format gives an example where a simple textual substitution with Y.Lang.sub is used to add text to the URL. This is wrong and may be dangerous if the substitution values are coming from the unsafe sources (such as parts of the URL – cgi parameters, hash etc.) because the insertion is done in the URL ((that is, into the URL context), and the injected data must be URI escaped. Consider this PoC (for time reasons, I skip the server side and just used example.com as JSONP target which does not provide any valid response – still, it should be sufficient to demonstrate the issue). We will take the replacement value from the location.hash.

The unsafe example code is this one: function prepareJSONPUrl(url, proxy, username) { return Y.Lang.sub(url, { callback: proxy, name: username || "user" // <<< THIS PART }); } http://dmitris.github.io/domxsstest/jsonptest.html#dmitris – this may be the intended usage, "dmitris" replaces the "name" placeholder, and the URL generated and fired is: http://example.com/dmitry/service.php?callback=YUI.Env.JSONP.yui_3_12_0_1_1378823866555_4

So far so good - but if the attacker sends a user the URL like: http://dmitris.github.io/domxsstest/jsonptest.html#evil?callback=alert(123) The YUI will generate and call the following URL: http://example.com/evil?callback=alert(123)/service.php?callback=YUI.Env.JSONP.yui_3_12_0_1_1378823817521_6

You can see that we were able to inject the "callback" CGI parameter (now there are two of them, and it depends on how the server handles the double arguments). Technically, it is called "HPP" - HTTP Parameter Pollution [1]

The vulnerability is fixed on this page: http://dmitris.github.io/domxsstest/jsonptest_fixed.html#evil?callback=alert%28123%29 Where the value added to the URL is wrapped in encodeURIComponent: function prepareJSONPUrl(url, proxy, username) { return Y.Lang.sub(url, { callback: proxy, name: encodeURIComponent (username) || "user" }); } In this case, all the injected data becomes just part of the path – probably, making it invalid – but there is only one '?' character and one callback CGI parameter with the intended value: http://example.com/evil%3Fcallback%3Dalert(123)/service.php?callback=YUI.Env.JSONP.yui_3_12_0_1_1378823936336_6

I hope it makes sense! :) We should fix the documentation to avoid people copying code from the official YUI documentation and creating vulnerabilities on their pages.

Here's the description of Stefano di Paola's talk: http://sched.appsec.eu/event/294be88bd5614ca31a83e2056404732f#.UhfiPhb52fQ , you can find the online recording on https://www.its.fh-muenster.de/owasp-appseceu13/rooms/Grosser_Saal/high_quality/OWASP-AppsecEU13-StefanoDiPaola-JavascriptlibrariesinsecurityAshowcaseofrecklessusesandunwittingmisuses_720p.mp4 He mentioned briefly this YUI case alongside of similar problems in other JavaScript libraries.

dmitris commented 11 years ago

I see now that it is already being worked on: https://github.com/yui/yui3/commit/66d93224c54944170f3b2642060a11c1664851bd https://github.com/yui/yui3/commit/b9c3e2863265484d19a124a89beecbcbd6c6c24b

Feel free to close (or leave it open until the changes make it in production). Thanks.

clarle commented 11 years ago

Hi @dmitris,

Yeah, we got the information from someone at the conference pretty quickly, but thanks for the report!

If you ever come across any other security problems in YUI, please contact us through our security contact e-mail.

Cheers!