Sanitizer::checkCss blacklist can be bypassed using vertical tab (ASCII 11)
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	PleaseStand
	Oct 5 2013, 6:47 AM

Description

I was reviewing gerrit 87648 and happened to find this vulnerability
in the existing code. The patch looks like it might fix the vulnerability (provided, of course, that all callers filter out unescaped vertical tabs), though I'm not sure whether that was the author's intent.

So it's possible that others already know about this vulnerability.

To reproduce (works in at least Firefox and Chromium), paste this wikitext in the edit box and hit Preview:

Actual Result: Logo is loaded from Google server and displayed
Expected Result: Logo should not be loaded from Google server or displayed. CSS should become /* insecure input */.

Version: unspecified
Severity: normal

Details

Reference: bz55332

Related Objects

Mentioned In: T232563: Drop IE6 and IE7 basic compatibility and security support
Mentioned Here: T232563: Drop IE6 and IE7 basic compatibility and security support

Event Timeline

• bzimport raised the priority of this task from to High.Nov 22 2014, 2:34 AM

• bzimport added a project: MediaWiki-Parser.

• bzimport set Reference to bz55332.

PleaseStand created this task.Oct 5 2013, 6:47 AM

It's worth noting that this may allow XSS (using things like behavior, -moz-binding, and JavaScript URLs), and also that there are many other ways to bypass blacklists like this one. For example, the following (adapted from http://html5sec.org/#css) works against IE 6:

Note that the property name "expression" is written in fullwidth characters.

On that page I also notice a couple properties -o-link and pointer-events that aren't blacklisted either.

(In reply to comment #1)

Note that the property name "expression" is written in fullwidth characters.

Fullwidth characters are characters like U+FF45 U+FF58 U+FF50 U+FF52 ...

I don't know why I wrote "property name" when "expression" isn't one.

(In reply to comment #1)

For example, the following (adapted from
http://html5sec.org/#css) works against IE 6:

And to further illustrate,

(using other characters such as U+0280 LATIN LETTER SMALL CAPITAL R,
as adapted from http://trac.edgewall.org/browser/trunk/trac/util/tests/html.py)
also works in IE 6.

(In reply to comment #0)

To reproduce (works in at least Firefox and Chromium), paste this wikitext in
the edit box and hit Preview:

<p style="font-size: 100px; background-image:
url\b(https://www.google.com/images/srpr/logo6w.png)">A</p>

Note: The "\b" attack only seems to work when $wgUseTidy = true; the "fullwidth" and "small caps" attacks against IE 6 work regardless of the $wgUseTidy setting.

Thanks Kevin. I knew our blacklist was missing a couple of properties, but I wasn't aware of the full-width characters or the \b. I'll start working up a patch for those next week. Please let us know if you find any other injections that get past that filter.

Created attachment 13452
Check for vertical tabs, fullwidth, and ipa unicode

attachment bug55332.patch ignored as obsolete

This patch should allow us to detect a url when it's followed by a vertical tab, as well as checking for the Fullwidth and IPA unicode attacks on IE6. Parser tests added too, based on the ones from http://trac.edgewall.org/browser/trunk/trac/util/tests/html.py

For the -o-link properties, Opera 11+ only allows those on <a> elements, which MediaWiki doesn't allow a way to create with styles. So it would only affect Opera 7-10, which seems to be a tiny fraction of our traffic[0]. I'm not sure if we ever have legitimate uses of that property-- if not, we can add it to the "/* insecure input */" blacklist.

I found a few ways to get around the patch (attachment 13452). Also note that PHP 5.2.3 (the MW 1.19 minimum requirement) does not support closures.

Superscript and subscript parentheses (IE 6)

<p style="font-size: 100px; background-image:url{CHAR}https://www.google.com/images/srpr/logo6w.png)">A</p>

Replace {CHAR} with one of:

U+207D SUPERSCRIPT LEFT PARENTHESIS
U+208D SUBSCRIPT LEFT PARENTHESIS

Various marks that repeat the S in expression (IE 6)

<p style="font-size: 100px; color: expres{CHAR}ion((title='XSSed'),'red')">B</p>

Replace {CHAR} with one of:

U+3031 VERTICAL KANA REPEAT MARK
U+309D HIRAGANA ITERATION MARK
U+30FC KATAKANA-HIRAGANA PROLONGED SOUND MARK
U+30FD KATAKANA ITERATION MARK
U+FE7C ARABIC SHADDA ISOLATED FORM
U+FE7D ARABIC SHADDA MEDIAL FORM
U+FF70 HALFWIDTH KATAKANA-HIRAGANA PROLONGED SOUND MARK

Everything except url( and expression (when $wgUseTidy is true)

No check for vertical tab there. Actually, the vertical tab is illegal in XML, and this function should probably not allow anything that is.

<div style="font-size: 100px; width: 200px; filter\b:progid:DXImageTransform.Microsoft.AlphaImageLoader(src='https://www.google.com/images/srpr/logo6w.png');">C</div>

(In reply to comment #8)

Everything except url( and expression (when $wgUseTidy is true)

Though this actually can affect IE 6 even when $wgUseTidy = false;

(In reply to comment #8)

I found a few ways to get around the patch (attachment 13452 [details]).
Also note that
PHP 5.2.3 (the MW 1.19 minimum requirement) does not support closures.

Superscript and subscript parentheses (IE 6)
<p style="font-size: 100px;
background-image:url{CHAR}https://www.google.com/images/srpr/logo6w.png)">A</
p>

Replace {CHAR} with one of:

U+207D SUPERSCRIPT LEFT PARENTHESIS

U+208D SUBSCRIPT LEFT PARENTHESIS

Various marks that repeat the S in expression (IE 6)
<p style="font-size: 100px; color:
expres{CHAR}ion((title='XSSed'),'red')">B</p>

Replace {CHAR} with one of:

U+3031 VERTICAL KANA REPEAT MARK

U+309D HIRAGANA ITERATION MARK

U+30FC KATAKANA-HIRAGANA PROLONGED SOUND MARK

U+30FD KATAKANA ITERATION MARK

U+FE7C ARABIC SHADDA ISOLATED FORM

U+FE7D ARABIC SHADDA MEDIAL FORM

U+FF70 HALFWIDTH KATAKANA-HIRAGANA PROLONGED SOUND MARK

Did you find those from a list of symbols that IE6 transliterates? Or are you finding these by testing? Since we're (for now) blacklisting, we do want to make sure we get them all. I'd like to rework our css handling to not use inline, and use something like htmlpurify to whitelist, but that's a long ways off.

Everything except url( and expression (when $wgUseTidy is true)

No check for vertical tab there. Actually, the vertical tab is illegal in
XML,
and this function should probably not allow anything that is.
<div style="font-size: 100px; width: 200px;
filter\b:progid:DXImageTransform.Microsoft.AlphaImageLoader(src='https://www.
google.com/images/srpr/logo6w.png');">C</div>

I'll add it to the forbidden control chars.

Created attachment 13475
IE 6 test script

Attached:

foojs.html2 KBDownload

(In reply to comment #10)

Did you find those from a list of symbols that IE6 transliterates? Or are you
finding these by testing? Since we're (for now) blacklisting, we do want to
make sure we get them all. I'd like to rework our css handling to not use
inline, and use something like htmlpurify to whitelist, but that's a long
ways
off.

By testing, so there are probably some I missed (in particular, I did not check for anything above U+FFFF). My test script replaced each of the characters with the HTML escape codes for everything in U+0000..U+FFFF. It did not replace more than one character at a time.

Furthermore, my test script seemed to give some false positives (e.g. even though I used <!DOCTYPE html>, the script reported that the equals sign could be substituted for the colon).

I ran your script under ie4linux'es version of ie6, and got back a lot more characters that were valid substitutions for expression. I verified that it gave a popup for each of these.

So here are the test cases I'm working against, although this is missing U+008A, U+009A, U+008C and U+009C, since I can't find a way to get those inserted into the input field in Chrome. I'll get a patch in shortly.

(In reply to comment #13)

I ran your script under ie4linux'es version of ie6, and got back a lot more
characters that were valid substitutions for expression. I verified that it
gave a popup for each of these.

(First of all, not everything marked "x:" is substituting for X.)

What you have looks like a pretty good match to the bestfit1251.txt and bestfit1252.txt lists (from http://www.unicode.org/Public/MAPPINGS/VENDORS/MICSFT/WindowsBestFit/) combined, though I haven't checked in detail.

It's odd that the superscript and subscript parentheses aren't in that list though they worked for me on a Windows XP VM (the one from the Microsoft site modern.ie). And outside the Halfwidth and Fullwidth Forms block, out of these, only U+207F seemed to be a problem on a real Windows XP installation (that isn't to say nobody else is running IE6 using Wine though). I suspect IE6 is using some Windows string function that is implemented differently in Wine.

I did make the code change the color instead of open an alert box to prevent endless alerting, though it didn't seem to make a difference; I just saw the same code points over and over again.

Alright, I think we should use the modern.ie list, since that should be a fully updated version of IE6. If someone is using an out of date version, I don't think we should be going too far out of our way to help them... they will have worse problems.

Created attachment 13534
Disallow vertical tabs, etc

attachment bug55332b.patch ignored as obsolete

A call to WideCharToMultiByte() with CodePage=CP_ACP and dwFlags not containing
WC_NO_BEST_FIT_CHARS would have this effect. I haven't identified the exact caller responsible, but such calls do seem to be common in IE.

http://msdn.microsoft.com/en-us/library/dd374130%28v=vs.85%29.aspx

"CP_ACP The system default Windows ANSI code page.

"Note This value can be different on different computers, even on the same network. It can be changed on the same computer, leading to stored data becoming irrecoverably corrupted. This value is only intended for temporary use and permanent storage should use UTF-16 or UTF-8 if possible."

Presumably CP1252 is most commonly used for CP_ACP, in which case bestfit1252.txt would be an appropriate reference.

Sorry, obviously bestfit1252.txt doesn't explain U+309D etc., and I've confirmed that Kevin's script shows U+309D as an issue on my Windows XP VM. Also, if it was plain CP1252 conversion, you would expect U+2320 and a few others to come up, which they don't. Maybe it is some sort of Unicode normalization step followed by CP1252 conversion.

I haven't gotten any further with my disassembly, so it's hard to know if the patch is comprehensive. But given the small number of trusted users using IE6, I think it is good enough to release.

Adding Gabriel, so he fix this in parsoid too.

Created attachment 13771
Move ss checks after comment stripping

Realized as I was backporting this I was checking for the repeated s before we stripped comments, so "expres/**/ゝion" would get through.

Attached:

bug55332c.patch4 KBDownload

Created attachment 13772
Patch for 1.19

Remove the closure for php 5.2 support

Attached:

bug55332c-119.patch5 KBDownload

Created attachment 13775
Patch for 1.20

Attached:

bug55332c-120.patch4 KBDownload

Created attachment 13776
Patch for 1.21

Attached:

bug55332c-121.patch4 KBDownload

CVE-2013-4567 - Vertical tab allows bypassing filters (all browsers)
CVE-2013-4568 - "expression" filtering in IE6 bypassed with non-ascii characters

Change 95545 merged by jenkins-bot:
SECURITY: Improve css javascript detection

https://gerrit.wikimedia.org/r/95545

Change 95542 merged by jenkins-bot:
SECURITY: Improve css javascript detection

https://gerrit.wikimedia.org/r/95542

Change 95557 had a related patch set uploaded by CSteipp:
SECURITY: Improve css javascript detection

https://gerrit.wikimedia.org/r/95557

Change 95538 merged by jenkins-bot:
SECURITY: Improve css javascript detection

https://gerrit.wikimedia.org/r/95538

Change 95557 merged by jenkins-bot:
SECURITY: Improve css javascript detection

https://gerrit.wikimedia.org/r/95557

No open patches to review here, hence restting status to RESOLVED FIXED.

Change 101290 had a related patch set uploaded by GWicke:
Fix css decoding in the sanitizer

https://gerrit.wikimedia.org/r/101290

Change 101290 merged by GWicke:
Fix css decoding in the sanitizer

https://gerrit.wikimedia.org/r/101290

Change 107301 had a related patch set uploaded by CSteipp:
SECURITY: Improve css javascript detection

https://gerrit.wikimedia.org/r/107301

Change 107301 merged by MarkAHershberger:
SECURITY: Improve css javascript detection

https://gerrit.wikimedia.org/r/107301

[Patch merged into REL1_22 branch; closing again]

• csteipp added a project: acl*security.Mar 26 2015, 8:39 PM

tstarling mentioned this in T232563: Drop IE6 and IE7 basic compatibility and security support.Sep 11 2019, 5:10 AM

In the course of backing out these changes for T232563, I confirmed the full-width, superscript parenthesis and S-repeat attacks against IE 6 and confirmed that they do not affect IE 8.

• chasemp added a project: Security.Feb 10 2020, 10:56 PM

• chasemp removed a project: acl*security.Feb 20 2020, 8:16 PM

	F12573: bug55332c-121.patch
	Nov 22 2014, 2:35 AM

	F12571: bug55332c-119.patch
	Nov 22 2014, 2:35 AM

	F12572: bug55332c-120.patch
	Nov 22 2014, 2:35 AM

	F12570: bug55332c.patch
	Nov 22 2014, 2:35 AM

Sanitizer::checkCss blacklist can be bypassed using vertical tab (ASCII 11)Closed, ResolvedPublicActions

Description

Details

Related Objects

Event Timeline

Superscript and subscript parentheses (IE 6)

Various marks that repeat the S in expression (IE 6)

Everything except url( and expression (when $wgUseTidy is true)

Superscript and subscript parentheses (IE 6)

Various marks that repeat the S in expression (IE 6)

Everything except url( and expression (when $wgUseTidy is true)

Sanitizer::checkCss blacklist can be bypassed using vertical tab (ASCII 11)
Closed, ResolvedPublic
Actions