Last Comment Bug 780979 - (CVE-2013-0760) Out-of-bounds-read in CharDistributionAnalysis::HandleOneChar
(CVE-2013-0760)
: Out-of-bounds-read in CharDistributionAnalysis::HandleOneChar
Status: RESOLVED FIXED
[asan][adv-main18+]
: sec-low, testcase
Product: Core
Classification: Components
Component: Internationalization (show other bugs)
: Trunk
: x86_64 All
: -- normal (vote)
: mozilla18
Assigned To: Andrew McCreight [:mccr8]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-07 13:12 PDT by Abhishek Arya
Modified: 2013-04-30 18:57 PDT (History)
7 users (show)
ryanvm: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
affected
affected
affected
fixed
wontfix


Attachments
Testcase (4.42 KB, application/x-zip-compressed)
2012-08-07 13:12 PDT, Abhishek Arya
no flags Details
compute the table lengths instead of hard coding them (5.70 KB, patch)
2012-08-15 11:32 PDT, Andrew McCreight [:mccr8]
no flags Details | Diff | Review
compute the length (8.57 KB, patch)
2012-08-28 14:50 PDT, Andrew McCreight [:mccr8]
smontagu: review+
Details | Diff | Review

Description Abhishek Arya 2012-08-07 13:12:40 PDT
Created attachment 649768 [details]
Testcase

Reproduces on trunk.


=================================================================
==30575== ERROR: AddressSanitizer global-buffer-overflow on address 0x7f058130ac5e at pc 0x7f0574303e43 bp 0x7fff591310d0 sp 0x7fff591310c8
READ of size 2 at 0x7f058130ac5e thread T0
    #0 0x7f0574303e43 in CharDistributionAnalysis::HandleOneChar(char const*, unsigned int) firefox/src/extensions/universalchardet/src/base/CharDistribution.h:37
    #1 0x7f05743099bd in nsEUCTWProber::HandleData(char const*, unsigned int) firefox/src/extensions/universalchardet/src/base/nsEUCTWProber.cpp:41
    #2 0x7f0574313ac2 in nsMBCSGroupProber::HandleData(char const*, unsigned int) firefox/src/extensions/universalchardet/src/base/nsMBCSGroupProber.cpp:125
    #3 0x7f05743297d6 in nsUniversalDetector::HandleData(char const*, unsigned int) firefox/src/extensions/universalchardet/src/base/nsUniversalDetector.cpp:181
    #4 0x7f05742f2125 in nsXPCOMDetector::DoIt(char const*, unsigned int, bool*) firefox/src/extensions/universalchardet/src/xpcom/nsUdetXPCOMWrapper.cpp:57
    #5 0x7f05742f281d in non-virtual thunk to nsXPCOMDetector::DoIt(char const*, unsigned int, bool*) asn1cmn.c:0
    #6 0x7f056cd55281 in nsContentUtils::GuessCharset(char const*, unsigned int, nsACString_internal&) firefox/src/content/base/src/nsContentUtils.cpp:3628
    #7 0x7f056ceab216 in nsDOMFileReader::GetAsText(nsACString_internal const&, char const*, unsigned int, nsAString_internal&) firefox/src/content/base/src/nsDOMFileReader.cpp:444
    #8 0x7f056ceaa7c4 in nsDOMFileReader::DoOnStopRequest(nsIRequest*, nsISupports*, unsigned int, nsAString_internal&, nsAString_internal&) firefox/src/content/base/src/nsDOMFileReader.cpp:357
    #9 0x7f056d5cc66c in mozilla::dom::FileIOObject::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) firefox/src/content/base/src/FileIOObject.cpp:206
    #10 0x7f056d5ccefd in non-virtual thunk to mozilla::dom::FileIOObject::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) asn1cmn.c:0
    #11 0x7f0569d80c4a in nsBaseChannel::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) firefox/src/netwerk/base/src/nsBaseChannel.cpp:720
    #12 0x7f0569d8102d in non-virtual thunk to nsBaseChannel::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) asn1cmn.c:0
    #13 0x7f0569e2384a in nsInputStreamPump::OnStateStop() firefox/src/netwerk/base/src/nsInputStreamPump.cpp:556
    #14 0x7f0569e203c5 in nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) firefox/src/netwerk/base/src/nsInputStreamPump.cpp:373
    #15 0x7f0569e23c4f in non-virtual thunk to nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) asn1cmn.c:0
    #16 0x7f05768e898e in nsInputStreamReadyEvent::Run() firefox/src/xpcom/io/nsStreamUtils.cpp:82
    #17 0x7f05769fcbfd in nsThread::ProcessNextEvent(bool, bool*) firefox/src/xpcom/threads/nsThread.cpp:625
    #18 0x7f057668b46d in NS_ProcessNextEvent_P(nsIThread*, bool) firefox/src/objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:220
    #19 0x7f0575540e96 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) firefox/src/ipc/glue/MessagePump.cpp:82
    #20 0x7f0576cb070a in MessageLoop::RunInternal() firefox/src/ipc/chromium/src/base/message_loop.cc:209
    #21 0x7f0576cb0553 in MessageLoop::RunHandler() firefox/src/ipc/chromium/src/base/message_loop.cc:202
    #22 0x7f0576cb0438 in MessageLoop::Run() firefox/src/ipc/chromium/src/base/message_loop.cc:176
    #23 0x7f05749f4f1e in nsBaseAppShell::Run() firefox/src/widget/xpwidgets/nsBaseAppShell.cpp:165
    #24 0x7f057363fb38 in nsAppStartup::Run() firefox/src/toolkit/components/startup/nsAppStartup.cpp:271
    #25 0x7f0569c40f60 in XREMain::XRE_mainRun() firefox/src/toolkit/xre/nsAppRunner.cpp:3798
    #26 0x7f0569c47902 in XREMain::XRE_main(int, char**, nsXREAppData const*) firefox/src/toolkit/xre/nsAppRunner.cpp:3875
    #27 0x7f0569c4add2 in XRE_main firefox/src/toolkit/xre/nsAppRunner.cpp:3951
    #28 0x40c28f in do_main(int, char**) firefox/src/browser/app/nsBrowserApp.cpp:174
    #29 0x409cbd in main firefox/src/browser/app/nsBrowserApp.cpp:279
    #30 0x7f0586e35c4d in ?? ??:0
0x7f058130ac5e is located 30 bytes to the right of global variable 'EUCTWCharToFreqOrder (firefox/src/extensions/universalchardet/src/base/CharDistribution.cpp)' (0x7f0581308240) of size 10752
==30575== ABORTING
Stats: 238M malloced (252M for red zones) by 382140 calls
Stats: 42M realloced by 19486 calls
Stats: 199M freed by 250896 calls
Stats: 66M really freed by 170600 calls
Stats: 440M (112717 full pages) mmaped in 110 calls
  mmaps   by size class: 8:278511; 9:40955; 10:16380; 11:14329; 12:2048; 13:2048; 14:1280; 15:256; 16:448; 17:1280; 18:128; 19:40; 20:16;
  mallocs by size class: 8:297871; 9:44986; 10:15483; 11:15613; 12:2123; 13:2131; 14:1393; 15:307; 16:536; 17:1485; 18:150; 19:48; 20:14;
  frees   by size class: 8:183583; 9:36302; 10:12152; 11:12696; 12:1450; 13:1216; 14:1203; 15:254; 16:453; 17:1472; 18:59; 19:45; 20:11;
  rfrees  by size class: 8:135364; 9:18346; 10:7215; 11:7785; 12:438; 13:437; 14:235; 15:144; 16:339; 17:256; 18:29; 19:11; 20:1;
Stats: malloc large: 1697 small slow: 1873
Shadow byte and word:
  0x1fe0b026158b: f9
  0x1fe0b0261588: f9 f9 f9 f9 00 00 00 00
More shadow bytes:
  0x1fe0b0261568: 00 00 00 00 00 00 00 00
  0x1fe0b0261570: 00 00 00 00 00 00 00 00
  0x1fe0b0261578: 00 00 00 00 00 00 00 00
  0x1fe0b0261580: 00 00 00 00 00 00 00 00
=>0x1fe0b0261588: f9 f9 f9 f9 00 00 00 00
  0x1fe0b0261590: 00 00 00 00 00 00 00 00
  0x1fe0b0261598: 00 00 00 00 00 00 00 00
  0x1fe0b02615a0: 00 00 00 00 00 00 00 00
  0x1fe0b02615a8: 00 00 00 00 00 00 00 00
Comment 1 Andrew McCreight [:mccr8] 2012-08-15 11:10:48 PDT
This is kind of a silly bug.  There are a bunch of static tables, with their length defined as a constant.  For some reason, the end of these tables was chopped off, but the length wasn't updated for EUCTWFreq.  EUCTW_TABLE_SIZE should be 5376, but it is 8102.  So the bounds check isn't right, and we end up reading off the end of the array.  At least that's what I think is happening from looking at the ASAN log, I haven't tried the test yet.

I think the way forward here is to replace these dangerous magic constants with a direct computation of the length.
Comment 2 Andrew McCreight [:mccr8] 2012-08-15 11:12:03 PDT
This file is untouched since at least 2007, so it affects everything.
Comment 3 Andrew McCreight [:mccr8] 2012-08-15 11:32:55 PDT
Created attachment 652157 [details] [diff] [review]
compute the table lengths instead of hard coding them
Comment 4 Simon Montagu :smontagu 2012-08-16 01:28:29 PDT
Do you want review for this or are you waiting on tests?
Comment 5 Andrew McCreight [:mccr8] 2012-08-16 08:03:39 PDT
(In reply to Simon Montagu from comment #4)
> Do you want review for this or are you waiting on tests?

I haven't gotten around to actually trying it out yet, beyond compiling it and running the browser.  There's no big hurry here, as I think we've kind of missed the window for the current cycle of beta, so I'm planning to wait until we're a bit into the next release cycle before landing it.
Comment 6 Andrew McCreight [:mccr8] 2012-08-28 14:50:52 PDT
Created attachment 656219 [details] [diff] [review]
compute the length
Comment 7 Andrew McCreight [:mccr8] 2012-08-29 07:00:20 PDT
Thanks!

https://hg.mozilla.org/integration/mozilla-inbound/rev/5420af81b778

I'm marking this sec-low.  The array length that is wrong is wrong by a fair amount, a few thousand, so you can read into some random stuff which I guess could cause a crash, but nothing is done with the value that is read out except increment a counter if the value found is > 512.  The data in the table is already fairly arbitrary, from a security standpoint, so it doesn't look to me like it could be that bad. Please say something or upgrade it if you disagree.
Comment 8 Ryan VanderMeulen [:RyanVM] 2012-08-29 17:23:22 PDT
https://hg.mozilla.org/mozilla-central/rev/5420af81b778
Comment 9 Lukas Blakk [:lsblakk] use ?needinfo 2012-08-30 16:32:06 PDT
sec-low, falls outside of esr criteria, wontfixing

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