780979 - (CVE-2013-0760) Out-of-bounds-read in CharDistributionAnalysis::HandleOneChar

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Abhishek Arya]

Attachment: 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]]

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]]

This file is untouched since at least 2007, so it affects everything.

Comment 3

[Andrew McCreight [:mccr8]]

Attachment: compute the table lengths instead of hard coding them

Comment 4

[Simon Montagu :smontagu]

Do you want review for this or are you waiting on tests?

Comment 5

[Andrew McCreight [:mccr8]]

(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]]

Attachment: compute the length

Comment 7

[Andrew McCreight [:mccr8]]

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]]

https://hg.mozilla.org/mozilla-central/rev/5420af81b778

Comment 9

[Lukas Blakk [:lsblakk] use ?needinfo]

sec-low, falls outside of esr criteria, wontfixing