stepmania / stepmania-site

StepMania's Website, forums, etc.
https://www.stepmania.com/
19 stars 7 forks source link

bbcode not escaping " (double quotes) properly #26

Closed SoonDead closed 10 years ago

SoonDead commented 10 years ago

It is possible to bust out of attributes now. It is bad.

See my signature for reference (more specifically the console where document.cookie is logged as proof of concept).

The only escaping I could find is str_replace(array('&', '<', '>'), array('&amp;', '&lt;', '&gt;') before applying bbcodes, and although adding " to the list would solve this case, I think this needs a more general solution.

  1. Before parsing escape all html entities.
  2. After parsing, purify the output with HTML Purifier

I'm not sure if the 2 points above are sufficient/necessary, so don't take my word for it. Some additional research is needed on the subject (e.g. how big forum engines deal with this, or best practices).

There is built-in bbcode support in php. Might worth to look at it.

Also the fix for javascript:blah is not really enough. It is case sensitive, there could be different kind of whitespace characters in the middle, etc. Although it is not that big of a concern, since most modern browsers prevent (does not execute) this kind of attack (it is so common).

shakesoda commented 10 years ago

Fixed in 588e00ca561b0f2a700c25b973c9f27b4feaec8e.

There was some terrible shortcoming in the built-in bbcode support, I think. I can't remember what. HTML Purifier might be a good idea at some point - but I don't think it's needed right now (although I'll probably reconsider when you have another pile of BBCode exploits here).

It might make it easier to add new BBCode without security concerns, though. That would be nice. I just don't want to throw filters on carelessly and inflate the server load without a real need.