Bugzilla@Mozilla – Bug 563618
Crash due to writing past the end of buffer [@ nsEUCJPToUnicodeV2::Convert]
Last modified: 2011-05-18 18:19:34 PDT
Summon comment box
No steps to reproduce; filed based on a crash stack. nsJapaneseToUnicode.cpp has these lines: 367 *dest++ = 0xFFFD; 368 // if 0x8e is not followed by a valid JIS X 0201 byte 369 // but by a valid US-ASCII, save it instead of eating it up. 370 if ( (PRUint8)*src < (PRUint8)0x7f ) 371 *dest++ = (PRUnichar) *src; The destination pointer is incremented on line 367 and then written to again on line 371 without first checking if the increment reached the end of the buffer. When the increment on line 367 reaches the end of the buffer, line 371 crashes. Even though this was already discussed on #developers, I'm marking this is as security sensitive in case the write past the buffer could be used to deliberately write something to the heap.
http://crash-stats.mozilla.com/report/index/ae6368f5-a6f8-4b7c-8f33-ef9102100429
This was a one-time only crash for me, I couldn't reproduce it. Iirc, I changed the charset a lot with the testcase.
So the input when it crashed was probably not actual EUC-JP?
Probably not, no. This was a crash occuring during fuzzing, which I couldn't reproduce.
Created attachment 443380 [details] testcase This test case doesn't reliably crash for me, but it does assert: ###!!! ASSERTION: The Unicode decoder wrote too much data.: 'mLastBuffer->getEnd() <= NS_HTML5_STREAM_PARSER_READ_BUFFER_SIZE', file /Users/simon/mozwork/hgtree/mozilla/parser/html/nsHtml5StreamParser.cpp, line 493
Created attachment 443713 [details] [review] Patch
Created attachment 443714 [details] [review] Tests
Comment on attachment 443713 [details] [review] Patch The change >- if ( ! (*src & 0xc0) ) >+ if ( (PRUint8)*src < (PRUint8)0x7f ) isn't related to the crash, but I think the original is wrong, maybe caused by copy-pasting code in the patch to bug 25037.
http://hg.mozilla.org/mozilla-central/rev/0841ec7eec87 http://hg.mozilla.org/mozilla-central/rev/93874cf6d6ba
We'd like to get this fix in for 1.9.2.14 and 1.9.1.17
Comment on attachment 443713 [details] [review] Patch a=LegNeato for 1.9.2.14 and 1.9.1.17
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/a5a94cd7b310 http://hg.mozilla.org/releases/mozilla-1.9.2/rev/a2ae8b0e65ee
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b576ae79792e http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ee5ea05fcd81
The crashtest passes for 1.9.1 and 1.9.2 on all platforms. Is there a reason that crashtests for a security bug were checked in prior to release, though? We normally hold those until after the release goes out.
The crashtests don't currently crash or assert on branch, so I think it's OK in this case.
(even without the patch, I mean)
Is there any way to verify the bug fix for branches?
Created attachment 507854 [details] [review] 1.8 version