Stupid XSS mistake, and why its so hard to write good code

Published: 2007-06-01
Last Updated: 2007-06-01 16:41:24 UTC
by Johannes Ullrich (Version: 1)
0 comment(s)
Stefan Esser (www.hardened-php.net) wrote in earlier to let me know about a XSS issue with the search function for the ISC website. Of course, while I respect his opinion and skills very much, I was first a bit sceptically (after all, I am not exactly suffering from low self esteem). However, he was helpful enough to provide a sample URL showing the problem.

So what happened and why? After all, I keep always saying that a web sites search function is the first spot to look for XSS.

Its pretty simple (and stupid). I pre-fill the search box with the last term a user searched for. This string is pulled from the users session, and the string is stored on the server. So I figured, I don't need to validate it. However, what I missed was that I didn't validate (Actually "escape" is better) right in the first place :-(.

The interesting part is that this issue hadn't been found before. The part that saved me was probably a generic validation for '<script>' tags that is performed on all user input. So defense in depth did work.

I do typically use a "safe_print" function, which is essentially just "print(htmlentities($string))". But well, I didn't use it in the header as some pages do not include the necessary library (you may call that another mistake. It saves a few CPU cycles and disk reads... but hurts you down the road).

Personally, I find that good code review is probably the hardest problem when it comes to these problems.  You just can't review your own code (at least I can't). If you want to help: much of the ISC code is available via Sourceforge (look for the "DShield" project).

Keywords:
0 comment(s)

Comments


Diary Archives