801681 - (CVE-2012-4207) "~" eats a char near chunk delimiter in HZ-GB-2312 encoding

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Masato Kinugawa]

User Agent: Mozilla/5.0 (Windows NT 6.0; rv:16.0) Gecko/20100101 Firefox/16.0
Build ID: 20121010144125

Steps to reproduce:

In HZ-GB-2312 encoding, "~" eats a char near chunk delimiter. This behavior is Firefox only. 

This leads to XSS attack:
http://vulnerabledoma.in/fx_hz?q=~%20123

Also, this has potential risk of Bug 690225.



Expected results:

"~" should not eat character.

Comment 1

[Daniel Veditz [:dveditz]]

Matt: please capture this testcase and add it as an attachment.

Comment 2

[Matt Wobensmith [:mwobensmith][:matt:]]

Attachment: Captured from http://vulnerabledoma.in/fx_hz?q=~%20123

Comment 3

[Daniel Veditz [:dveditz]]

Attachment: captured testcase

More faithful capture... the previous attachment suffered from browser translation of >< to &gt;&lt; which broke the testcase -- onmouseover triggered because it was legitimately in the source regardless of charset.

If you load this testcase as the hz-gb-2312 charset you see one <input> with an alert triggered on mouseover. If you then go to the View menu and change the Character Encoding to UTF-8 you will see two inputs, with "~ 123" in one and the onmouseover injection in the other.

Comment 4

[Daniel Veditz [:dveditz]]

Comment on attachment 672572 [details]
captured testcase

This doesn't work either. The only obvious difference I see is that vulnerabledoma.in is using Transfer-Encoding: chunked and there's no way to do that for an attached testcase that I know of.

Comment 5

[Simon Montagu :smontagu]

Attachment: Alternative testcase

The bug is there in the decoder even without the chunking - this testcase finds another way to exhibit it.

Comment 6

[Simon Montagu :smontagu]

Attachment: Patch

When unconsuming the character we need to decrement the loop index as well as the source pointer.

The patch fixes another error, not related to this bug: we should only increment iDestlen++ when actually outputting a character. I need to work out a testcase for that as well.

Comment 7

[Simon Montagu :smontagu]

https://hg.mozilla.org/integration/mozilla-inbound/rev/38784da3792d

Comment 8

[Simon Montagu :smontagu]

Attachment: Testcase 2

This is a testcase for the second issue mentioned in comment 6. Without the patch it asserts:
 
###!!! ASSERTION: The Unicode decoder consumed the wrong number of bytes.: 'totalByteCount == (int32_t)aCount', file parser/html/nsHtml5StreamParser.cpp, line 869
###!!! ASSERTION: Wrong number of stream bytes written/sniffed.: 'writeCount == aLength', file parser/html/nsHtml5StreamParser.cpp, line 1077

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/38784da3792d

Comment 10

[Simon Montagu :smontagu]

Comment on attachment 672714 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 715319
User impact if declined: Possibility of XSS attack in pages encoded in HZ-GB-2312
Testing completed (on m-c, etc.): On m-c since 2012-10-18
Risk to taking this patch (and alternatives if risky): minimal
String or UUID changes made by this patch: None

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/82fddf7e0497
https://hg.mozilla.org/releases/mozilla-beta/rev/e3d6c1a5fa41
https://hg.mozilla.org/releases/mozilla-esr10/rev/17e10f1d5063

Comment 13

[Simon Montagu :smontagu]

Tests checked in https://hg.mozilla.org/integration/mozilla-inbound/rev/f27d5d9ebef2

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f27d5d9ebef2

Comment 15

[Tracy Walker [:tracy]]

mass remove verifyme requests greater than 4 months old