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.
--
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [
http://daniel.friesen.name]