966006 - (CVE-2014-1532) heap-use-after-free in libxul.so!nsHostResolver::ConditionallyRefreshRecord()

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Tyson Smith [:tsmith]]

Found by the BlackBerry Security Automated Analysis Team's fuzzing framework ALF.

At this time we do not have a test case that will reproduce the issue.

==14607==ERROR: AddressSanitizer: heap-use-after-free on address 0x603001988ce0 at pc 0x7fb8e190676a bp 0x7fff84db1080 sp 0x7fff84db1078
READ of size 4 at 0x603001988ce0 thread T0
    #0 0x7fb8e1906769 (libxul.so!nsHostResolver::ConditionallyRefreshRecord(nsHostRecord*, char const*)+0x1a9)
	Line 368 of "../../dist/include/nsTArray.h"
    #1 0x7fb8e190533d (libxul.so!nsHostResolver::ResolveHost(char const*, unsigned short, unsigned short, nsResolveHostCallback*)+0xd2d)
	Line 634 of "/builds/slave/m-in-l64-asan-0000000000000000/build/netwerk/dns/nsHostResolver.cpp"
    #2 0x7fb8e1917874 (libxul.so!nsDNSService::AsyncResolve(nsACString_internal const&, unsigned int, nsIDNSListener*, nsIEventTarget*, nsICancelable**)+0xb44)
	Line 675 of "/builds/slave/m-in-l64-asan-0000000000000000/build/netwerk/dns/nsDNSService2.cpp"
    #3 0x7fb8e182cc9d (libxul.so!nsDNSPrefetch::Prefetch(unsigned short)+0x1dd)
	Line 64 of "/builds/slave/m-in-l64-asan-0000000000000000/build/netwerk/base/src/nsDNSPrefetch.cpp"
    #4 0x7fb8e1ae05bf (libxul.so!mozilla::net::nsHttpChannel::BeginConnect()+0x104f)
	Line 4540 of "/builds/slave/m-in-l64-asan-0000000000000000/build/netwerk/protocol/http/nsHttpChannel.cpp"
    #5 0x7fb8e1ae15be (libxul.so!mozilla::net::nsHttpChannel::OnProxyAvailable(nsICancelable*, nsIURI*, nsIProxyInfo*, tag_nsresult)+0x10e)
	Line 4650 of "/builds/slave/m-in-l64-asan-0000000000000000/build/netwerk/protocol/http/nsHttpChannel.cpp"
    #6 0x7fb8e18b2a25 (libxul.so!nsAsyncResolveRequest::DoCallback()+0x9a5)
	Line 238 of "/builds/slave/m-in-l64-asan-0000000000000000/build/netwerk/base/src/nsProtocolProxyService.cpp"
    #7 0x7fb8e1878251 (libxul.so!nsProtocolProxyService::AsyncResolveInternal(nsIURI*, unsigned int, nsIProtocolProxyCallback*, nsICancelable**, bool)+0x8a1)
	Line 130 of "/builds/slave/m-in-l64-asan-0000000000000000/build/netwerk/base/src/nsProtocolProxyService.cpp"
    #8 0x7fb8e1ad03d9 (libxul.so!mozilla::net::nsHttpChannel::ResolveProxy()+0x2f9)
	Line 1776 of "/builds/slave/m-in-l64-asan-0000000000000000/build/netwerk/protocol/http/nsHttpChannel.cpp"
    #9 0x7fb8e1adf3ea (libxul.so!mozilla::net::nsHttpChannel::AsyncOpen(nsIStreamListener*, nsISupports*)+0x5ea)
	Line 4444 of "/builds/slave/m-in-l64-asan-0000000000000000/build/netwerk/protocol/http/nsHttpChannel.cpp"
    #10 0x7fb8e4ea2a59 (libxul.so!mozilla::dom::HTMLTrackElement::LoadResource()+0xad9)
	Line 247 of "/builds/slave/m-in-l64-asan-0000000000000000/build/content/html/content/src/HTMLTrackElement.cpp"
    #11 0x7fb8e4ea858c (libxul.so!nsRunnableMethodImpl<void (mozilla::dom::HTMLTrackElement::*)(), void, true>::Run()+0x6c)
	Line 383 of "../../../../dist/include/nsThreadUtils.h"
    #12 0x7fb8e4e36415 (libxul.so!mozilla::dom::nsSyncSection::Run()+0xa5)
	Line 693 of "/builds/slave/m-in-l64-asan-0000000000000000/build/content/html/content/src/HTMLMediaElement.cpp"
    #13 0x7fb8e4007dc8 (libxul.so!nsBaseAppShell::RunSyncSectionsInternal(bool, unsigned int)+0x228)
	Line 349 of "/builds/slave/m-in-l64-asan-0000000000000000/build/widget/xpwidgets/nsBaseAppShell.cpp"
    #14 0x7fb8e40085d8 (libxul.so!non-virtual thunk to nsBaseAppShell::AfterProcessNextEvent(nsIThreadInternal*, unsigned int, bool)+0x48)
	Line 90 of "/builds/slave/m-in-l64-asan-0000000000000000/build/widget/xpwidgets/nsBaseAppShell.h"
    #15 0x7fb8e16f47da (libxul.so!nsThread::ProcessNextEvent(bool, bool*)+0xffa)
	Line 648 of "/builds/slave/m-in-l64-asan-0000000000000000/build/xpcom/threads/nsThread.cpp"
    #16 0x7fb8e15ce521 (libxul.so!NS_ProcessNextEvent(nsIThread*, bool)+0xb1)
	Line 263 of "/builds/slave/m-in-l64-asan-0000000000000000/build/xpcom/glue/nsThreadUtils.cpp"
    #17 0x7fb8e1eedbd1 (libxul.so!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)+0x311)
	Line 95 of "/builds/slave/m-in-l64-asan-0000000000000000/build/ipc/glue/MessagePump.cpp"
    #18 0x7fb8e1e5fd53 (libxul.so!MessageLoop::Run()+0x1c3)
	Line 226 of "/builds/slave/m-in-l64-asan-0000000000000000/build/ipc/chromium/src/base/message_loop.cc"
    #19 0x7fb8e400696c (libxul.so!nsBaseAppShell::Run()+0x5c)
	Line 161 of "/builds/slave/m-in-l64-asan-0000000000000000/build/widget/xpwidgets/nsBaseAppShell.cpp"
    #20 0x7fb8e6c4c2c6 (libxul.so!nsAppStartup::Run()+0xc6)
	Line 268 of "/builds/slave/m-in-l64-asan-0000000000000000/build/toolkit/components/startup/nsAppStartup.cpp"
    #21 0x7fb8e6a646e5 (libxul.so!XREMain::XRE_mainRun()+0x1de5)
	Line 4023 of "/builds/slave/m-in-l64-asan-0000000000000000/build/toolkit/xre/nsAppRunner.cpp"
    #22 0x7fb8e6a6561a (libxul.so!XREMain::XRE_main(int, char**, nsXREAppData const*)+0x4fa)
	Line 4091 of "/builds/slave/m-in-l64-asan-0000000000000000/build/toolkit/xre/nsAppRunner.cpp"
    #23 0x7fb8e6a6654b (libxul.so!XRE_main+0x3ab)
	Line 4331 of "/builds/slave/m-in-l64-asan-0000000000000000/build/toolkit/xre/nsAppRunner.cpp"
    #24 0x459dcd (firefox!main+0x94d)
	Line 280 of "/builds/slave/m-in-l64-asan-0000000000000000/build/browser/app/nsBrowserApp.cpp"
    #25 0x7fb8f1c2d76c (libc.so.6!__libc_start_main+0xec)
	Line 226 of "libc-start.c"
    #26 0x45934c (firefox!_start+0x28)
0x603001988ce0 is located 0 bytes inside of 24-byte region [0x603001988ce0,0x603001988cf8)
freed by thread T3 (Socket Thread) here:
    #0 0x446255 (firefox!free+0x55)
	Line 64 of "/builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc"
    #1 0x7fb8e15c2a5d (libxul.so!nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyWithMemutils>::ShrinkCapacity(unsigned int, unsigned long)+0x14d)
	Line 212 of "/builds/slave/m-in-l64-asan-0000000000000000/build/xpcom/build/../glue/nsTArray.h"
previously allocated by thread T3 (Socket Thread) here:
    #0 0x446395 (firefox!malloc+0x55)
	Line 74 of "/builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc"
    #1 0x7fb8ed308588 (libmozalloc.so!moz_xmalloc+0x8)
	Line 52 of "/builds/slave/m-in-l64-asan-0000000000000000/build/memory/mozalloc/mozalloc.cpp"
Thread T3 (Socket Thread) created by T0 here:
    #0 0x437801 (firefox!pthread_create+0x71)
	Line 155 of "/builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc"
    #1 0x7fb8ef6d9be5 (libnspr4.so!_PR_CreateThread+0x4a5)
	Line 445 of "/builds/slave/m-in-l64-asan-0000000000000000/build/nsprpub/pr/src/pthreads/ptthread.c"
    #2 0x7fb8ef6d9737 (libnspr4.so!PR_CreateThread+0x17)
	Line 528 of "/builds/slave/m-in-l64-asan-0000000000000000/build/nsprpub/pr/src/pthreads/ptthread.c"
Shadow bytes around the buggy address:
  0x0c0680329140: fa fa fa fa fa fa fa fa fd fd fd fd fa fa fd fd
  0x0c0680329150: fd fd fa fa fa fa fa fa fa fa fd fd fd fd fa fa
  0x0c0680329160: fa fa fa fa fa fa 00 00 00 fa fa fa fa fa fa fa
  0x0c0680329170: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fd fd
  0x0c0680329180: fd fd fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c0680329190: fa fa fa fa fa fa fd fd fd fa fa fa[fd]fd fd fa
  0x0c06803291a0: fa fa fa fa fa fa fa fa fd fd fd fd fa fa 00 00
  0x0c06803291b0: 02 fa fa fa fd fd fd fa fa fa fd fd fd fd fa fa
  0x0c06803291c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c06803291d0: fa fa fa fa fa fa fa fa 00 00 00 00 fa fa fd fd
  0x0c06803291e0: fd fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:     fa
  Heap right redzone:    fb
  Freed heap region:     fd
  Stack left redzone:    f1
  Stack mid redzone:     f2
  Stack right redzone:   f3
  Stack partial redzone: f4
  Stack after return:    f5
  Stack use after scope: f8
  Global redzone:        f9
  Global init order:     f6
  Poisoned by user:      f7
  ASan internal:         fe
==14607==ABORTING

Comment 1

[Andrew McCreight [:mccr8]]

Jason, is there somebody who could take a look at these stacks and see if there's something obvious to fix?  It seems unlikely, but you never know.

Comment 2

[Jason Duell]

Nick, IIRC you know this code somewhat. Can you take a look?  It's a fuzzer crash so it's likely to be non-intuitive.

Comment 3

[Jason Duell]

> At this time we do not have a test case that will reproduce the issue.

Tyson, is there anything we can do to change that?  A test case would be golden here.

Comment 4

[u408661]

So my best guess so far is that nsHostResolver::ConditionallyRefreshRecord touches rec->mBlacklistedItems without holding rec->addr_info_lock. It's not obvious that the lock needs to be held when accessing mBlacklistedItems, but there is a note that certain functions that modify mBlacklistedItems should only be called while holding the lock, so it's possible we're racing against one of those.

Is this easy enough to reproduce under the fuzzer (ie, without an explicit test case) to validate a build with a fix, or was this a so-far-only-one-time kind of thing?

Comment 5

[Matt Wobensmith [:mwobensmith][:matt:]]

Tyson, do you have any feedback for Nicolas' question in comment 4?

Comment 6

[Tyson Smith [:tsmith]]

Oh sorry I missed that, we have seen this issue 3x. So I don't think we would be a great verification method.

Comment 7

[u408661]

Attachment: 0001-Bug-966006.-r-mcmanus.patch

Comment 8

[Patrick McManus [:mcmanus]]

(In reply to Nicholas Hurley [:hurley] from comment #4)
> So my best guess so far is that nsHostResolver::ConditionallyRefreshRecord
> touches rec->mBlacklistedItems without holding rec->addr_info_lock. It's not

I think that's very likely the right diagnosis. Thanks Nick!

The implications seem low risk (if we don't segv we basically end up getting a random answer to a question on whether or not to honor the cache)

Comment 9

[u408661]

Comment on attachment 8387768 [details] [diff] [review]
0001-Bug-966006.-r-mcmanus.patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch? Probably not very, but I've been wrong before

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? Doubtful.

Which older supported branches are affected by this flaw? All.

If not all supported branches, which bug introduced the flaw? N/A

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Don't have backports, but they'll be easy to create, and probably not very risky.

How likely is this patch to cause regressions; how much testing does it need? Not very likely, though it could expose something previously hidden. If the trees are happy after landing, that should be all the testing necessary (or possible, given how rarely this has been reproduced)

Comment 10

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

Comment on attachment 8387768 [details] [diff] [review]
0001-Bug-966006.-r-mcmanus.patch

It is too late in the cycle to take this since we're shipping a week from tomorrow.

This has sec-approval+ to go in on April 1 or later (two weeks into the next six week cycle).

We'll want patches for Aurora, Beta, and ESR24 after it goes into trunk.

Comment 11

[David Bolter [:davidb] (NeedInfo me for attention)]

Hi Nicholas can you land this on trunk? That is the next step as per comment 10. We can do branch landings after that, hopefully you can prepare those?

Comment 12

[u408661]

(In reply to David Bolter [:davidb] from comment #11)
> Hi Nicholas can you land this on trunk? That is the next step as per comment
> 10. We can do branch landings after that, hopefully you can prepare those?

Will do, just waiting for inbound to open (unless we want to shove this straight onto m-c). In the meantime, I'll prepare patches for the other affected branches (should be the same, as that code doesn't change much at all)

Comment 13

[u408661]

Attachment: patch for esr24

But of course, esr24 has diverged on this code path (the crashing function was part of another function at this point). It's really the same patch, just in a slightly different location.

Comment 14

[u408661]

Well, good(?) news... it seems that *only* esr24 is affected by this bug now, since bug 981513 (and the followup in bug 981447) landed. The racy access that caused the crash no longer exists on m-c, m-a, OR m-b, so all we need is the patch for esr24. As such, I won't be landing anything for this bug until that patch gets reviewed and approved :)

Comment 15

[u408661]

Comment on attachment 8402811 [details] [diff] [review]
patch for esr24

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: possible crashes (or worse nastiness), likelihood is uncertain (only happened 3 times under a fuzzer, otherwise not reproduced to my knowledge)
Fix Landed on Version: esr24 is the only affected version, so nowhere
Risk to taking this patch (and alternatives if risky): low
String or UUID changes made by this patch: none

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 16

[u408661]

remote:   https://hg.mozilla.org/releases/mozilla-esr24/rev/cd6cf5fa4d1d

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/82e5ff16645a
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/d598c2c64b45

Comment 18

[Matt Wobensmith [:mwobensmith][:matt:]]

Marking [qa-], no test case.