Looking over an extension that was already badly coded, I realized there's yet another type of injection vulnerability we have to consider when coding. CSS injection vulnerabilities.
Normally MediaWiki sanitizes any style="" tag created by user input. Things like background-image's are stripped out. They can be used to track users, as a type of spam, and if you're hitting IE users it's possible you could do even more using a htc file. Oh right, and of course there's the lovely ie expression(...) which allows raw JavaScript to be injected right into css.
There are some extensions however that may try to build style strings for their own html output. For example an extension trying to output something where it just happens to want to let the user customize the width and height of a div it happens to be outputting. To do so the extension likely will concatenate the user's input into some css using something like `"width: {$width}; height: {$height};` the problem here of course is that if the user's input included something like `width="5px; background-image: url(...)"` they could use it to inject a background image that would not be filtered out by our normal code. And just a note, yes extensions using our nice elegant Html interface to try to avoid normal XSS injections can still be bitten by this: If you use something like `Html::element( 'div', array( 'style' => "width: {$width}; height: {$height};" ), '...' );` then it can still be used.
Things needed to fix issues like this: - Any style tag you generate using user input should be properly sanitized using Sanitizer::checkCss - Keep in mind that checkCss only rejects things like url structures expressions, etc... it does not ensure that only the properties that you specified are used. So it's possible that users can still do less malicious things like use a width="500px; position: absolute; ..." to inject other css into your style. I suppose it's already possible to absolutely position a flash video somewhere it's not supposed to be by wrapping a div outside of it so that's not really as big a deal. Though if you have css styles or some special case in your code where you might not want a style to be overridden or added keep in mind that a user can still inject those properties. -- We should probably consider building an interface such as `Html::element( 'div', array( 'style' => array( 'width' => $width, 'height' => $height ) ), '...' );` into core.
Am 16.09.2011 01:12, schrieb Daniel Friesen:
Looking over an extension that was already badly coded, I realized there's yet another type of injection vulnerability we have to consider when coding. CSS injection vulnerabilities.
Normally MediaWiki sanitizes any style="" tag created by user input. Things like background-image's are stripped out. They can be used to track users, as a type of spam, and if you're hitting IE users it's possible you could do even more using a htc file. Oh right, and of course there's the lovely ie expression(...) which allows raw JavaScript to be injected right into css.
Daniel,
please can you add the essentials of your important information regarding CSS injection vulnerabilities via extensions to the relevant pages in the MediaWiki Developer's Guide
http://www.mediawiki.org/wiki/MDG
I guess, your information should be added to some pages in section Security and some pages in section Extensions, SpecialPages, hooks & Co.,
Tom
Hey,
We should probably consider building an interface such as `Html::element(
'div', array( 'style' => array( 'width' => $width, 'height' => $height ) ), '...' );` into core.
+1; I actually didn't read this email very carefully and changed this in some extensions before realizing it's a proposed interface and not an existing one :)
Cheers
-- Jeroen De Dauw http://www.bn2vs.com Don't panic. Don't be evil. --
wikitech-l@lists.wikimedia.org