744541 - (CVE-2012-1947) Heap-buffer-overflow in utf16_to_isolatin1

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Abhishek Arya]

Attachment: firefox-crash.html

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/535.11 (KHTML, like Gecko) Chrome/17.0.963.83 Safari/535.11

Steps to reproduce:

I ran the attached testcase on ASANified Firefox Beta 12.0 and Firefox Aurora 13.0a2 and got this crash. The testcase reproduces reliably on slower bots.

=================================================================
==22842== ERROR: AddressSanitizer heap-buffer-overflow on address 0x7f3963484b86 at pc 0x7f397f1c4155 bp 0x7fff6ca8a7a0 sp 0x7fff6ca8a798
READ of size 2 at 0x7f3963484b86 thread T0
    #0 0x7f397f1c4155 in utf16_to_isolatin1 firefox/beta-src/xpcom/io/nsNativeCharsetUtils.cpp:123
    #1 0x7f397f1c470e in NS_CopyUnicodeToNative(nsAString_internal const&, nsACString_internal&) firefox/beta-src/xpcom/io/nsNativeCharsetUtils.cpp:865
    #2 0x7f397f1e2acf in nsLocalFile::InitWithPath(nsAString_internal const&) firefox/beta-src/xpcom/io/nsLocalFileUnix.cpp:1968
    #3 0x7f397d3c6345 in net_GetFileFromURLSpec(nsACString_internal const&, nsIFile**) firefox/beta-src/netwerk/base/src/nsURLHelperUnix.cpp:121
    #4 0x7f397d39dfed in nsStandardURL::EnsureFile() firefox/beta-src/netwerk/base/src/nsStandardURL.cpp:2546
    #5 0x7f397d39e06f in nsStandardURL::GetFile(nsIFile**) firefox/beta-src/netwerk/base/src/nsStandardURL.cpp:2553
    #6 0x7f397d463c2f in nsFileChannel::OpenContentStream(bool, nsIInputStream**, nsIChannel**) firefox/beta-src/netwerk/protocol/file/nsFileChannel.cpp:315
    #7 0x7f397d349de0 in nsBaseChannel::BeginPumpingData() firefox/beta-src/netwerk/base/src/nsBaseChannel.cpp:240
    #8 0x7f397d34bd59 in nsBaseChannel::AsyncOpen(nsIStreamListener*, nsISupports*) firefox/beta-src/netwerk/base/src/nsBaseChannel.cpp:609
    #9 0x7f397dcf38e9 in nsScriptLoader::StartLoad(nsScriptLoadRequest*, nsAString_internal const&) firefox/beta-src/content/base/src/nsScriptLoader.cpp:339
    #10 0x7f397dcf9bb9 in nsScriptLoader::PreloadURI(nsIURI*, nsAString_internal const&, nsAString_internal const&) firefox/beta-src/content/base/src/nsScriptLoader.cpp:1340
    #11 0x7f397e3a02a2 in nsHtml5TreeOpExecutor::PreloadScript(nsAString_internal const&, nsAString_internal const&, nsAString_internal const&) firefox/beta-src/parser/html/nsHtml5TreeOpExecutor.cpp:940
    #12 0x7f397e3aa949 in nsHtml5SpeculativeLoad::Perform(nsHtml5TreeOpExecutor*) firefox/beta-src/parser/html/nsHtml5SpeculativeLoad.cpp:67
    #13 0x7f397e39ea54 in nsHtml5TreeOpExecutor::RunFlushLoop() firefox/beta-src/parser/html/nsHtml5TreeOpExecutor.cpp:478
    #14 0x7f397e3aa2f2 in nsHtml5ExecutorFlusher::Run() firefox/beta-src/parser/html/nsHtml5StreamParser.cpp:160
    #15 0x7f397f165d7d in NS_ProcessNextEvent_P(nsIThread*, bool) firefox/beta-src/objdir-ff-asan/xpcom/build/nsThreadUtils.cpp:245
    #16 0x7f397efe5b25 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) firefox/beta-src/ipc/glue/MessagePump.cpp:110
    #17 0x7f397f26bccf in MessageLoop::Run() firefox/beta-src/ipc/chromium/src/base/message_loop.cc:176
    #18 0x7f397edf3acc in nsBaseAppShell::Run() firefox/beta-src/widget/xpwidgets/nsBaseAppShell.cpp:191
    #19 0x407982 in do_main firefox/beta-src/browser/app/nsBrowserApp.cpp:205
    #20 0x40707a in main firefox/beta-src/browser/app/nsBrowserApp.cpp:295
    #21 0x7f3984550c4d in __libc_start_main /build/buildd/eglibc-2.11.1/csu/libc-start.c:258
0x7f3963484b86 is located 0 bytes to the right of 262-byte region [0x7f3963484a80,0x7f3963484b86)
allocated by thread T0 here:
    #0 0x4103e2 in malloc 
    #1 0x7f397f234a9d in nsStringBuffer::Alloc(unsigned long) firefox/beta-src/xpcom/string/src/nsSubstring.cpp:209
    #2 0x7f397f237467 in nsAString_internal::SetCapacity(unsigned int) firefox/beta-src/xpcom/string/src/nsTSubstring.cpp:549
    #3 0x7f397f235b9d in nsAString_internal::SetLength(unsigned int) firefox/beta-src/xpcom/string/src/nsTSubstring.cpp:579
==22842== ABORTING
Stats: 98M malloced (100M for red zones) by 213591 calls
Stats: 5M realloced by 10131 calls
Stats: 68M freed by 115413 calls
Stats: 0M really freed by 0 calls
Stats: 220M (56351 full pages) mmaped in 55 calls
  mmaps   by size class: 8:180213; 9:32764; 10:12285; 11:12282; 12:2048; 13:1536; 14:1280; 15:256; 16:512; 17:128; 18:64; 19:8; 20:4; 21:2;
  mallocs by size class: 8:159655; 9:26841; 10:10317; 11:11443; 12:1809; 13:1402; 14:1249; 15:204; 16:479; 17:123; 18:57; 19:6; 20:4; 21:2;
  frees   by size class: 8:75941; 9:18459; 10:7803; 11:9347; 12:1445; 13:636; 14:1116; 15:170; 16:364; 17:115; 18:11; 19:4; 20:2;
  rfrees  by size class:
Stats: malloc large: 192 small slow: 1306
Shadow byte and word:
  0x1fe72c690970: 6
  0x1fe72c690970: 06 fb fb fb fb fb fb fb
More shadow bytes:
  0x1fe72c690950: 00 00 00 00 00 00 00 00
  0x1fe72c690958: 00 00 00 00 00 00 00 00
  0x1fe72c690960: 00 00 00 00 00 00 00 00
  0x1fe72c690968: 00 00 00 00 00 00 00 00
=>0x1fe72c690970: 06 fb fb fb fb fb fb fb
  0x1fe72c690978: fb fb fb fb fb fb fb fb
  0x1fe72c690980: fa fa fa fa fa fa fa fa
  0x1fe72c690988: fa fa fa fa fa fa fa fa
  0x1fe72c690990: fa fa fa fa fa fa fa fa

Comment 1

[Christian Holler (:decoder)]

So far, I was neither able to reproduce this on trunk nor on aurora. Is there anything special that one needs to do for running?

Comment 2

[Abhishek Arya]

I was able to reproduce it on Aurora, as I said in c#0. I did now find why it wont reproduce on certain machines.

Check out this code

if (IsUTF8(path)) {
        // speed up the start-up where UTF-8 is the native charset
        // (e.g. on recent Linux distributions)
        if (NS_IsNativeUTF8())
            rv = localFile->InitWithNativePath(path);
        else
            rv = localFile->InitWithPath(NS_ConvertUTF8toUTF16(path));
            // XXX In rare cases, a valid UTF-8 string can be valid as a native 
            // encoding (e.g. 0xC5 0x83 is valid both as UTF-8 and Windows-125x).
            // However, the chance is very low that a meaningful word in a legacy
            // encoding is valid as UTF-8.
    }

On my bots, LANG env variable was not set. So it reproduced there. On my local machine, it was set to utf8, so it didnt reproduce there. Just run 'unset LANG' and then the testcase reproduces easily :)

Comment 3

[Mats Palmgren (inactive)]

I can reproduce this with "unset LANG" in an asan debug build on Linux64,
Thanks Abhishek!

The bug is a buffer overflow caused by nsNativeCharsetConverter::UnicodeToNative(const PRUnichar **input, ...)
when the iconv() call fails.  It's calling the fallback function
utf16_to_isolatin1() at the end with the original length params, but with
the buffer pointers that the failed iconv() call have changed.

http://hg.mozilla.org/mozilla-central/annotate/d373afa244ee/xpcom/io/nsNativeCharsetUtils.cpp#l565

I pushed this to Try:
https://hg.mozilla.org/try/rev/25f5de6b955e

That handles the failure in the same way as nsNativeCharsetConverter::NativeToUnicode(const char **input, ...),
which is to update the length params to where iconv() stopped.

http://hg.mozilla.org/mozilla-central/annotate/d373afa244ee/xpcom/io/nsNativeCharsetUtils.cpp#l494

I think it would be safer to make a copy of the all the param values and
restore them before calling the fallback function, so we don't rely on
what state iconv() has left them in... do we trust inconv() to behave
exactly the same in error cases on all platforms?

Comment 4

[Mats Palmgren (inactive)]

Code archeology puts the blame on Darin, the interesting bit is that we
actually fixed this bug in the "char**" variant in bug 225125, but nobody
seems to have bothered to look for more occurrences even though the
"PRUnichar**" variant of the same function is just below it!!! 

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/xpcom%2Fio%2FnsNativeCharsetUtils.cpp#554

Comment 5

[Mats Palmgren (inactive)]

FWIW, this code is only compiled for XP_UNIX, excluding
#if defined(XP_MACOSX) || defined(ANDROID).  So only Linux/Unix/BSD etc
are affected.  The buffer overflow is nasty because it overwrites the stack
(char temp[4096]; in NS_CopyUnicodeToNative).

I haven't found any way to reach this code from http: content though.
Most calls deals with local file names, which are prohibited of course,
(the test only crashes when using file: )

There are a couple of calls that possibly could be reached from remote
sources, I don't know how they are used:
./security/manager/ssl/src/nsNTLMAuthModule.cpp:
  NS_CopyUnicodeToNative(username, oemUserBuf);
./extensions/auth/nsAuthSASL.cpp:
  NS_CopyUnicodeToNative(mUsername, userbuf);

It's also used in toolkit/xre/nsConsoleWriter.cpp but that's not enabled
by default (browser.dom.window.dump.enabled), right?  Bug 467013 and
bug 608994 appears to be about that call site.

Comment 6

[Mats Palmgren (inactive)]

Attachment: crashtest

Comment 7

[Mats Palmgren (inactive)]

Attachment: fix

Comment 8

[Boris Zbarsky [:bzbarsky]]

Don't we need to handle the gUnicodeToNative == INVALID_ICONV_T case as in bug 225125?

Comment 9

[Mats Palmgren (inactive)]

Yes, the potential else-branch is the 2nd hunk.  This patch makes UnicodeToNative
pretty much a mirror image of NativeToUnicode, so I'm not sure what you mean.

Comment 10

[Mats Palmgren (inactive)]

Attachment: same patch with larger context

Comment 11

[Boris Zbarsky [:bzbarsky]]

Ah, OK.  I was comparing to the actual diff in bug 225125 which sets some out params up front before the if.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 615069 [details] [diff] [review]
fix

r=me

Comment 13

[Mats Palmgren (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6c0fdf2079a9

Comment 14

[Mats Palmgren (inactive)]

(intentionally holding the crashtest until the fix is deployed on relevant branches)

Comment 15

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/6c0fdf2079a9

Comment 17

[Daniel Veditz [:dveditz]]

Rating "sg:moderate" due to the risk reduction of affecting only debug builds or file: urls.

Comment 19

[Al Billings [:abillings - ex-MoCo]]

Christian, can you verify this is fixed on trunk?

Comment 20

[Christian Holler (:decoder)]

(In reply to Al Billings [:abillings] from comment #19)
> Christian, can you verify this is fixed on trunk?

Unfortunately I haven't been able to reproduce this in the first place. Maybe Mats can verify if he has a working build/way to reproduce?

Comment 21

[Bob Clary [:bc] (inactive)]

I haven't seen the signatures from Bug 608994 on Nightly/14 since this landed though I have seen them on Beta/12 and Aurora/13.

Comment 22

[Mats Palmgren (inactive)]

Comment on attachment 615069 [details] [diff] [review]
fix

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: [sg:moderate] likely exploitable, but only on Linux/Unix/BSD, and likely only from file: urls (unless window.dump is enabled)
Testing completed (on m-c, etc.): on m-c since 2012-04-18, on Aurora since the uplift
Risk to taking this patch (and alternatives if risky): low risk since it's a fairly simple patch and it's easy to see that the code change only affects
the path when iconv() failed (res == -1), which is a path very rarely taken.
String changes made by this patch: none

Comment 23

[Alex Keybl [:akeybl]]

Comment on attachment 615069 [details] [diff] [review]
fix

[Triage Comment]
Given the low risk evaluation and possible exploitability, approving for Beta 13 and the ESR branch.

Comment 24

[Mats Palmgren (inactive)]

https://hg.mozilla.org/releases/mozilla-esr10/rev/bf90c6d4fd49
https://hg.mozilla.org/releases/mozilla-beta/rev/382eea58c0f4

Comment 25

[u279076]

Created an ASAN build based off latest-mozilla-central (Firefox 15) and the testcase does not crash. I'm not sure this is enough to call it VERIFIED, and whether we can assume this is fixed for other branches (in particular Beta/ESR) given we can't make ASAN builds there yet.

Comment 26

[Al Billings [:abillings - ex-MoCo]]

Given how hard it was to see this bug in action, it is probably not going to be easy to verify.

Comment 27

[Bob Clary [:bc] (inactive)]

fwiw, i haven't seen bug 608994 on beta/13, aurora/14, nightly/15 in some time.