Last Comment Bug 801681 - (CVE-2012-4207) "~" eats a char near chunk delimiter in HZ-GB-2312 encoding
(CVE-2012-4207)
: "~" eats a char near chunk delimiter in HZ-GB-2312 encoding
Status: RESOLVED FIXED
[adv-track-main17+][adv-track-esr17+]...
: regression, sec-high, testcase
Product: Core
Classification: Components
Component: Internationalization (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla19
Assigned To: Simon Montagu :smontagu
:
Mentors:
Depends on:
Blocks: CVE-2012-0471
  Show dependency treegraph
 
Reported: 2012-10-15 08:38 PDT by Masato Kinugawa
Modified: 2014-07-24 14:39 PDT (History)
9 users (show)
dveditz: sec‑bounty+
smontagu: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
fixed
fixed
fixed
17+
fixed
fixed


Attachments
Captured from http://vulnerabledoma.in/fx_hz?q=~%20123 (182 bytes, text/html;charset=hz-gb-2312)
2012-10-17 11:06 PDT, Matt Wobensmith [:mwobensmith][:matt:]
no flags Details
captured testcase (60 bytes, text/html;charset=hz-gb-2312)
2012-10-17 16:06 PDT, Daniel Veditz [:dveditz]
no flags Details
Alternative testcase (278 bytes, text/html; charset=hz-gb-2312)
2012-10-18 01:28 PDT, Simon Montagu :smontagu
no flags Details
Patch (1.34 KB, patch)
2012-10-18 01:56 PDT, Simon Montagu :smontagu
VYV03354: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
lukasblakk+bugs: approval‑mozilla‑esr10+
Details | Diff | Splinter Review
Testcase 2 (152 bytes, text/html)
2012-10-18 13:08 PDT, Simon Montagu :smontagu
no flags Details

Description Masato Kinugawa 2012-10-15 08:38:46 PDT
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] 2012-10-17 10:57:01 PDT
Matt: please capture this testcase and add it as an attachment.
Comment 2 Matt Wobensmith [:mwobensmith][:matt:] 2012-10-17 11:06:39 PDT
Created attachment 672400 [details]
Captured from http://vulnerabledoma.in/fx_hz?q=~%20123
Comment 3 Daniel Veditz [:dveditz] 2012-10-17 16:06:09 PDT
Created attachment 672572 [details]
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] 2012-10-17 16:12:07 PDT
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 2012-10-18 01:28:11 PDT
Created attachment 672713 [details]
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 2012-10-18 01:56:25 PDT
Created attachment 672714 [details] [diff] [review]
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 8 Simon Montagu :smontagu 2012-10-18 13:08:14 PDT
Created attachment 672922 [details]
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] 2012-10-18 18:47:38 PDT
https://hg.mozilla.org/mozilla-central/rev/38784da3792d
Comment 10 Simon Montagu :smontagu 2012-10-20 23:35:12 PDT
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 13 Simon Montagu :smontagu 2013-02-13 22:23:12 PST
Tests checked in https://hg.mozilla.org/integration/mozilla-inbound/rev/f27d5d9ebef2
Comment 14 Ed Morley [:emorley] 2013-02-14 02:54:06 PST
https://hg.mozilla.org/mozilla-central/rev/f27d5d9ebef2
Comment 15 Tracy Walker [:tracy] 2014-01-10 10:40:48 PST
mass remove verifyme requests greater than 4 months old

Note You need to log in before you can comment on or make changes to this bug.