recalapp / recal

First a COS 333 project, now a very popular tool at Princeton for course selection
http://recal.io
MIT License
12 stars 3 forks source link

Security fixes #221

Closed maximz closed 10 years ago

maximz commented 10 years ago

Vulnerabilities

SQL Injection attacks:

We're protected since we only use Django's ORM.

XSS attacks:

We need to be very careful here. An easy way this could be exploited is if someone types <a href="javascript: alert('hi')"> in any field of an event popup. This would be executed on everyone else's machine, unless we properly escape it!

Note: as of before this pull request, we're vulnerable to this attack!

Templates handle escaping automatically, but we don't use templates often for AJAX calls. The solution is to use templates more explicitly. Untrusted data must be properly encoded!

When to sanitize? Per http://diovo.com/2008/09/sanitizing-user-data-how-and-where-to-do-it/, http://stackoverflow.com/a/130323/130164, and http://stackoverflow.com/q/1274558/130164: sanitize SQL that's going into the database (i.e. remove quotes) -- Django ORM handles this -- and sanitize HTML coming out of the database (to prevent XSS).

Thus, we just need to add sanitization on the output. Input is fine.

CSRF attacks:

All GET requests should be free of side effects; only POST calls should make state changes. We seem to have this down for the most part -- except for /user-logout for example -- but it's not that big a deal since everything except /chromeframe cannot be embedded in an iframe and used maliciously.

We also need to check Host and Referrer headers for our POST requests.

Last, we need to include CSRF tokens in all our POST requests. Since we're writing a one-page app, we need to load the CSRF token into JS, include the current token in each POST, and then fetch a new one after every POST. Challenging but worthwhile.

Progress