ieb / timetables

Timetables
http://www.caret.cam.ac.uk/blogs/ucam-timetables/
GNU Affero General Public License v3.0
2 stars 1 forks source link

Audit status message allows arbitary HTML, is vulnerable to XSS #31

Open h4l opened 12 years ago

h4l commented 12 years ago

Arbitrary HTML can be specified for an audit status message. For example, one could set their status message as:

This is a message with some <b>bold</b> text which executes some javascript: <script type="text/javascript">alert("hi");</script>

Which results in the javascript in the script tag being executed for anyone viewing the status message.

Sadly there are almost certainly many other instances of such vulnerability in the codebase.

h4l commented 12 years ago

To be fixed later when resources allow (there are most likely lots of simliar issues in the code). Considered lower than normal priority as user must be granted privileged access rights to exploit.

ieb commented 12 years ago

Could we pass all input in all posts through an xss filter [1] or do we need the user to be able to submit more than the most basic html markup ?

I am assuming that there are no GET parameters that get through to the output ?

http://stackoverflow.com/questions/1336776/xss-filtering-function-in-php

h4l commented 12 years ago

In principle yes. I can't think of any reason why users would need to submit any javascript etc.

One possible issue is that at least some form submissions are not really be using forms as forms. E.g. the editor POSTs form data with one key, the value is a JSON document. In these sorts of cases we'd have to be careful not to damage the JSON.

ieb commented 12 years ago

Thats going to be problematic and hard to do in an automated way since the json could easilly contain HTML and how do we know what the intention was.

In most real web frameworks you dont have to think about this. HTML escaping is on by default and you have to disable it by exception. Django does this by default. I think the best way of solving this is to fix the transition from data to HTML as we have no real idea where the data comes from.

I am seriously wondering if it would not be easier to simply port the whole of the web app to Django and address all the potential vulnerabilities in a single sweep, there are bound to be more.

h4l commented 12 years ago

That would certainly be a good option. There's nothing special about the PHP layer that would warrant keeping it in my opinion.