Closed Bug 976922 Opened 6 years ago Closed 6 years ago

heap-use-after-free mozilla::net::CacheEntry::GetMetaDataElement in NS_strdup

Categories

(Core :: Networking: Cache, defect, critical)

30 Branch
x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla30
Tracking Status
firefox28 --- unaffected
firefox29 --- disabled
firefox30 + fixed
firefox-esr24 --- unaffected

People

(Reporter: tsmith, Assigned: michal)

Details

(Keywords: crash, csectype-uaf, sec-critical, Whiteboard: [qa-])

Attachments

(1 file)

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.

==28295==ERROR: AddressSanitizer: heap-use-after-free on address 0x60b0009eca31 at pc 0x7fc810b7572e bp 0x7fc803537210 sp 0x7fc803537208
READ of size 66 at 0x60b0009eca31 thread T8 (Cache2 I/O)
    #0 0x7fc810b7572d (libxul.so!NS_strdup(char const*)+0xfd)
	Line 120 of "/builds/slave/m-in-l64-asan-0000000000000000/build/xpcom/glue/nsCRTGlue.cpp"
    #1 0x7fc810fb9707 (libxul.so!mozilla::net::CacheEntry::GetMetaDataElement(char const*, char**)+0xd7)
	Line 1147 of "/builds/slave/m-in-l64-asan-0000000000000000/build/netwerk/cache2/CacheEntry.cpp"
    #2 0x7fc8110da616 (libxul.so!mozilla::net::nsHttpChannel::OnCacheEntryCheck(nsICacheEntry*, nsIApplicationCache*, unsigned int*)+0x726)
	Line 2694 of "/builds/slave/m-in-l64-asan-0000000000000000/build/netwerk/protocol/http/nsHttpChannel.cpp"
    #3 0x7fc810fb5c87 (libxul.so!mozilla::net::CacheEntry::InvokeCallback(mozilla::net::CacheEntry::Callback&)+0x3b7)
	Line 599 of "/builds/slave/m-in-l64-asan-0000000000000000/build/netwerk/cache2/CacheEntry.cpp"
    #4 0x7fc810fb5414 (libxul.so!mozilla::net::CacheEntry::InvokeCallbacks(bool)+0x4f4)
	Line 535 of "/builds/slave/m-in-l64-asan-0000000000000000/build/netwerk/cache2/CacheEntry.cpp"
    #5 0x7fc810fb3709 (libxul.so!mozilla::net::CacheEntry::InvokeCallbacks()+0x49)
	Line 489 of "/builds/slave/m-in-l64-asan-0000000000000000/build/netwerk/cache2/CacheEntry.cpp"
    #6 0x7fc810fb3d03 (libxul.so!mozilla::net::CacheEntry::BackgroundOp(unsigned int, bool)+0x593)
	Line 1508 of "/builds/slave/m-in-l64-asan-0000000000000000/build/netwerk/cache2/CacheEntry.cpp"
    #7 0x7fc810fba217 (libxul.so!non-virtual thunk to mozilla::net::CacheEntry::Run()+0x67)
	Line 1294 of "/builds/slave/m-in-l64-asan-0000000000000000/build/netwerk/cache2/CacheEntry.cpp"
    #8 0x7fc811006292 (libxul.so!mozilla::net::CacheIOThread::LoopOneLevel(unsigned int)+0x232)
	Line 220 of "/builds/slave/m-in-l64-asan-0000000000000000/build/netwerk/cache2/CacheIOThread.cpp"
    #9 0x7fc811005b8d (libxul.so!mozilla::net::CacheIOThread::ThreadFunc()+0x21d)
	Line 156 of "/builds/slave/m-in-l64-asan-0000000000000000/build/netwerk/cache2/CacheIOThread.cpp"
    #10 0x7fc81f195009 (libnspr4.so!_pt_root+0x519)
	Line 212 of "/builds/slave/m-in-l64-asan-0000000000000000/build/nsprpub/pr/src/pthreads/ptthread.c"
    #11 0x44cf03 (firefox-bin!__asan::AsanThread::ThreadStart(unsigned long)+0x83)
	Line 138 of "/builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_thread.cc"
    #12 0x7fc8226ace99 (libpthread.so.0!start_thread+0xd9)
	Line 308 of "pthread_create.c"
    #13 0x7fc8217b73fc (libc.so.6!clone+0x6c)
	Line 112 of "../sysdeps/unix/sysv/linux/x86_64/clone.S"
0x60b0009eca31 is located 33 bytes inside of 100-byte region [0x60b0009eca10,0x60b0009eca74)
freed by thread T0 here:
    #0 0x44653b (firefox-bin!realloc+0x5b)
	Line 95 of "/builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc"
    #1 0x7fc81b635da8 (libmozalloc.so!moz_xrealloc+0x8)
	Line 84 of "/builds/slave/m-in-l64-asan-0000000000000000/build/memory/mozalloc/mozalloc.cpp"
previously allocated by thread T0 here:
    #0 0x44653b (firefox-bin!realloc+0x5b)
	Line 95 of "/builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc"
    #1 0x7fc81b635da8 (libmozalloc.so!moz_xrealloc+0x8)
	Line 84 of "/builds/slave/m-in-l64-asan-0000000000000000/build/memory/mozalloc/mozalloc.cpp"
Thread T8 (Cache2 I/O) created by T0 here:
    #0 0x437801 (firefox-bin!pthread_create+0x71)
	Line 155 of "/builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc"
    #1 0x7fc81f190ea2 (libnspr4.so!_PR_CreateThread+0x4a2)
	Line 453 of "/builds/slave/m-in-l64-asan-0000000000000000/build/nsprpub/pr/src/pthreads/ptthread.c"
    #2 0x7fc81f1909f7 (libnspr4.so!PR_CreateThread+0x17)
	Line 544 of "/builds/slave/m-in-l64-asan-0000000000000000/build/nsprpub/pr/src/pthreads/ptthread.c"
Shadow bytes around the buggy address:
  0x0c16801358f0: 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa fa
  0x0c1680135900: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1680135910: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1680135920: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1680135930: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c1680135940: fa fa fd fd fd fd[fd]fd fd fd fd fd fd fd fd fa
  0x0c1680135950: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1680135960: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1680135970: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1680135980: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c1680135990: 00 fa fa fa fa fa fa fa fa fa 00 00 00 00 00 00
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
Severity: normal → critical
Status: UNCONFIRMED → NEW
Ever confirmed: true
Any idea here Honza?  Even if we landed the new cache ASAP, we'll this fixed for aurora/beta and ESR if possible.
Flags: needinfo?(honzab.moz)
Michal, seem like I get the released memory (bad pointer) from CacheFile (you), any idea?  Could mBuf and mElementsSize in CacheFileMetadata be broken?  Personally I kinda wonder why you haven't implemented metadata key/values rather as a hashtable, would be more reliable..

This can only happen with cache2 on!
Flags: needinfo?(honzab.moz) → needinfo?(michal.novotny)
Hmm, there is a bug. You got a correct pointer from CacheFile::GetElement(), but the buffer in CacheFileMetadata is protected by CacheFile's lock, so the memory can be released just after GetElement() finishes. We need to duplicate the memory in CacheFile under its lock.
Assignee: nobody → michal.novotny
Flags: needinfo?(michal.novotny)
Attached patch fixSplinter Review
Attachment #8386123 - Flags: review?(honzab.moz)
Attachment #8386123 - Flags: review?(honzab.moz) → review+
https://hg.mozilla.org/mozilla-central/rev/544d3e2f8f7b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Can someone confirm that 29 is unaffected?
The code is not executed in 29 unless somebody explicitly turns on the new cache backend.
OK. Do you think it would make sense to also uplift the fix to 29 then?
(In reply to Sylvestre Ledru [:sylvestre] from comment #9)
> OK. Do you think it would make sense to also uplift the fix to 29 then?

The patch is simple and safe.  I'm not against.
If it does not take more than a few minutes, it would be nice to have it.
Marking [qa-] w/r/t verification, no test case.
Whiteboard: [qa-]
Group: core-security
You need to log in before you can comment on or make changes to this bug.