Closed Bug 1397702 Opened 7 years ago Closed 7 years ago

Intermittent SUMMARY: AddressSanitizer: heap-use-after-free /builds/worker/workspace/build/src/xpcom/ds/PLDHashTable.h:228:26 in Get

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 + fixed
firefox57 + fixed

People

(Reporter: aryx, Assigned: baku)

References

(Blocks 1 open bug)

Details

(Keywords: csectype-uaf, intermittent-failure, sec-high, Whiteboard: [post-critsmash-triage])

Attachments

(1 file)

https://treeherder.mozilla.org/logviewer.html#?job_id=129182306&repo=autoland

[task 2017-09-07T10:06:16.458113Z] 10:06:16     INFO - TEST-START | browser/components/sessionstore/test/browser_cookies_privacy.js
[task 2017-09-07T10:06:16.708517Z] 10:06:16     INFO - GECKO(1101) | =================================================================
[task 2017-09-07T10:06:16.711565Z] 10:06:16    ERROR - GECKO(1101) | ==1101==ERROR: AddressSanitizer: heap-use-after-free on address 0x60700007f8d8 at pc 0x7f9242291330 bp 0x7ffd35b997c0 sp 0x7ffd35b997b8
[task 2017-09-07T10:06:16.715957Z] 10:06:16     INFO - GECKO(1101) | READ of size 8 at 0x60700007f8d8 thread T0
[task 2017-09-07T10:06:20.210829Z] 10:06:20     INFO - GECKO(1101) |     #0 0x7f924229132f in Get /builds/worker/workspace/build/src/xpcom/ds/PLDHashTable.h:228:26
[task 2017-09-07T10:06:20.213849Z] 10:06:20     INFO - GECKO(1101) |     #1 0x7f924229132f in PLDHashTable::Iterator::Iterator(PLDHashTable*) /builds/worker/workspace/build/src/xpcom/ds/PLDHashTable.cpp:728
[task 2017-09-07T10:06:20.247477Z] 10:06:20     INFO - GECKO(1101) |     #2 0x7f9247bb4072 in Iterator /builds/worker/workspace/build/src/obj-firefox/dist/include/nsBaseHashtable.h:345:50
[task 2017-09-07T10:06:20.248893Z] 10:06:20     INFO - GECKO(1101) |     #3 0x7f9247bb4072 in Iter /builds/worker/workspace/build/src/obj-firefox/dist/include/nsBaseHashtable.h:363
[task 2017-09-07T10:06:20.250526Z] 10:06:20     INFO - GECKO(1101) |     #4 0x7f9247bb4072 in mozilla::dom::SessionStorageManager::ClearStorages(mozilla::dom::SessionStorageManager::ClearStorageType, mozilla::OriginAttributesPattern const&, nsACString const&) /builds/worker/workspace/build/src/dom/storage/SessionStorageManager.cpp:210
[task 2017-09-07T10:06:20.268361Z] 10:06:20     INFO - GECKO(1101) |     #5 0x7f9247bb4b0d in mozilla::dom::SessionStorageManager::Observe(char const*, nsAString const&, nsACString const&) /builds/worker/workspace/build/src/dom/storage/SessionStorageManager.cpp:255:5
[task 2017-09-07T10:06:20.277616Z] 10:06:20     INFO - GECKO(1101) |     #6 0x7f9247bd676d in Notify /builds/worker/workspace/build/src/dom/storage/StorageObserver.cpp:145:11
[task 2017-09-07T10:06:20.279155Z] 10:06:20     INFO - GECKO(1101) |     #7 0x7f9247bd676d in mozilla::dom::StorageObserver::Observe(nsISupports*, char const*, char16_t const*) /builds/worker/workspace/build/src/dom/storage/StorageObserver.cpp:219
[task 2017-09-07T10:06:20.282149Z] 10:06:20     INFO - GECKO(1101) |     #8 0x7f92422b109c in nsObserverList::NotifyObservers(nsISupports*, char const*, char16_t const*) /builds/worker/workspace/build/src/xpcom/ds/nsObserverList.cpp:112:19
[task 2017-09-07T10:06:20.284175Z] 10:06:20     INFO - GECKO(1101) |     #9 0x7f92422b49e8 in nsObserverService::NotifyObservers(nsISupports*, char const*, char16_t const*) /builds/worker/workspace/build/src/xpcom/ds/nsObserverService.cpp:296:19
[task 2017-09-07T10:06:20.322868Z] 10:06:20     INFO - GECKO(1101) |     #10 0x7f9242643676 in nsCookieService::NotifyChanged(nsISupports*, char16_t const*, bool) /builds/worker/workspace/build/src/netwerk/cookie/nsCookieService.cpp:2256:7
[task 2017-09-07T10:06:20.325043Z] 10:06:20     INFO - GECKO(1101) |     #11 0x7f9242644528 in nsCookieService::RemoveAll() /builds/worker/workspace/build/src/netwerk/cookie/nsCookieService.cpp:2380:3
[task 2017-09-07T10:06:20.327901Z] 10:06:20     INFO - GECKO(1101) |     #12 0x7f92423cd021 in NS_InvokeByIndex /builds/worker/workspace/build/src/xpcom/reflect/xptcall/md/unix/xptcinvoke_asm_x86_64_unix.S:129
[task 2017-09-07T10:06:20.351994Z] 10:06:20     INFO - GECKO(1101) |     #13 0x7f9243b7ca00 in Invoke /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1996:12
[task 2017-09-07T10:06:20.355052Z] 10:06:20     INFO - GECKO(1101) |     #14 0x7f9243b7ca00 in Call /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1315
[task 2017-09-07T10:06:20.359664Z] 10:06:20     INFO - GECKO(1101) |     #15 0x7f9243b7ca00 in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1282
[task 2017-09-07T10:06:20.361215Z] 10:06:20     INFO - GECKO(1101) |     #16 0x7f9243b839cf in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:928:12
[task 2017-09-07T10:06:20.397273Z] 10:06:20     INFO - GECKO(1101) |     #17 0x7f924d024d94 in CallJSNative /builds/worker/workspace/build/src/js/src/jscntxtinlines.h:293:15
[task 2017-09-07T10:06:20.399443Z] 10:06:20     INFO - GECKO(1101) |     #18 0x7f924d024d94 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:494
[task 2017-09-07T10:06:20.408409Z] 10:06:20     INFO - GECKO(1101) |     #19 0x7f924d00e6a2 in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:545:12
[task 2017-09-07T10:06:20.412044Z] 10:06:20     INFO - GECKO(1101) |     #20 0x7f924d00e6a2 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3093
[task 2017-09-07T10:06:20.416868Z] 10:06:20     INFO - GECKO(1101) |     #21 0x7f924cff5f6b in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:434:12
[task 2017-09-07T10:06:20.421390Z] 10:06:20     INFO - GECKO(1101) |     #22 0x7f924d024f2c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:512:15
[task 2017-09-07T10:06:20.423397Z] 10:06:20     INFO - GECKO(1101) |     #23 0x7f924d025882 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:558:10
[task 2017-09-07T10:06:20.441891Z] 10:06:20     INFO - GECKO(1101) |     #24 0x7f924d81cb63 in js::jit::InterpretResume(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/jit/VMFunctions.cpp:941:12
[task 2017-09-07T10:06:20.452020Z] 10:06:20     INFO - GECKO(1101) |     #25 0x7f91fb2ff6b2  (<unknown module>)
[task 2017-09-07T10:06:20.461049Z] 10:06:20     INFO - GECKO(1101) | 0x60700007f8d8 is located 56 bytes inside of 72-byte region [0x60700007f8a0,0x60700007f8e8)
[task 2017-09-07T10:06:20.462487Z] 10:06:20     INFO - GECKO(1101) | freed by thread T0 here:
[task 2017-09-07T10:06:20.504957Z] 10:06:20     INFO - GECKO(1101) |     #0 0x4bb6fb in __interceptor_free /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:47:3
[task 2017-09-07T10:06:20.511545Z] 10:06:20     INFO - GECKO(1101) |     #1 0x7f9247bb1524 in mozilla::dom::SessionStorageManager::Release() /builds/worker/workspace/build/src/dom/storage/SessionStorageManager.cpp:18:1
[task 2017-09-07T10:06:20.623020Z] 10:06:20     INFO - GECKO(1101) |     #2 0x7f924bf9d32a in ~nsCOMPtr_base /builds/worker/workspace/build/src/obj-firefox/dist/include/nsCOMPtr.h:313:7
[task 2017-09-07T10:06:20.625284Z] 10:06:20     INFO - GECKO(1101) |     #3 0x7f924bf9d32a in nsDocShell::~nsDocShell() /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp:906
[task 2017-09-07T10:06:20.626735Z] 10:06:20     INFO - GECKO(1101) |     #4 0x7f924bf9ed0d in nsDocShell::~nsDocShell() /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp:879:1
[task 2017-09-07T10:06:20.635778Z] 10:06:20     INFO - GECKO(1101) |     #5 0x7f9243d5ed45 in nsDocLoader::Release() /builds/worker/workspace/build/src/uriloader/base/nsDocLoader.cpp:173:1
[task 2017-09-07T10:06:20.638247Z] 10:06:20     INFO - GECKO(1101) | previously allocated by thread T0 here:
[task 2017-09-07T10:06:20.641857Z] 10:06:20     INFO - GECKO(1101) |     #0 0x4bba4c in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64:3
[task 2017-09-07T10:06:20.659292Z] 10:06:20     INFO - GECKO(1101) |     #1 0x4ecf6d in moz_xmalloc /builds/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:84:17
[task 2017-09-07T10:06:20.687524Z] 10:06:20     INFO - GECKO(1101) |     #2 0x7f924997252c in operator new /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:206:12
[task 2017-09-07T10:06:20.689203Z] 10:06:20     INFO - GECKO(1101) |     #3 0x7f924997252c in SessionStorageManagerConstructor(nsISupports*, nsID const&, void**) /builds/worker/workspace/build/src/layout/build/nsLayoutModule.cpp:252
[task 2017-09-07T10:06:20.692542Z] 10:06:20     INFO - GECKO(1101) |     #4 0x7f924235e561 in nsComponentManagerImpl::CreateInstanceByContractID(char const*, nsISupports*, nsID const&, void**) /builds/worker/workspace/build/src/xpcom/components/nsComponentManager.cpp:1086:19
[task 2017-09-07T10:06:20.709346Z] 10:06:20     INFO - GECKO(1101) |     #5 0x7f9242363edd in CallCreateInstance /builds/worker/workspace/build/src/xpcom/components/nsComponentManagerUtils.cpp:149:38
[task 2017-09-07T10:06:20.711131Z] 10:06:20     INFO - GECKO(1101) |     #6 0x7f9242363edd in nsCreateInstanceByContractID::operator()(nsID const&, void**) const /builds/worker/workspace/build/src/xpcom/components/nsComponentManagerUtils.cpp:197
[task 2017-09-07T10:06:20.756665Z] 10:06:20     INFO - GECKO(1101) | SUMMARY: AddressSanitizer: heap-use-after-free /builds/worker/workspace/build/src/xpcom/ds/PLDHashTable.h:228:26 in Get
[task 2017-09-07T10:06:20.759229Z] 10:06:20     INFO - GECKO(1101) | Shadow bytes around the buggy address:
[task 2017-09-07T10:06:20.763702Z] 10:06:20     INFO - GECKO(1101) |   0x0c0e80007ec0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa 00 00
[task 2017-09-07T10:06:20.765500Z] 10:06:20     INFO - GECKO(1101) |   0x0c0e80007ed0: 00 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa
[task 2017-09-07T10:06:20.767318Z] 10:06:20     INFO - GECKO(1101) |   0x0c0e80007ee0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
[task 2017-09-07T10:06:20.769504Z] 10:06:20     INFO - GECKO(1101) |   0x0c0e80007ef0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
[task 2017-09-07T10:06:20.773532Z] 10:06:20     INFO - GECKO(1101) |   0x0c0e80007f00: 00 fa fa fa fa fa 00 00 00 00 00 00 00 00 02 fa
[task 2017-09-07T10:06:20.775325Z] 10:06:20     INFO - GECKO(1101) | =>0x0c0e80007f10: fa fa fa fa fd fd fd fd fd fd fd[fd]fd fa fa fa
[task 2017-09-07T10:06:20.777234Z] 10:06:20     INFO - GECKO(1101) |   0x0c0e80007f20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
[task 2017-09-07T10:06:20.782515Z] 10:06:20     INFO - GECKO(1101) |   0x0c0e80007f30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa 00 00
[task 2017-09-07T10:06:20.787539Z] 10:06:20     INFO - GECKO(1101) |   0x0c0e80007f40: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
[task 2017-09-07T10:06:20.789546Z] 10:06:20     INFO - GECKO(1101) |   0x0c0e80007f50: 00 00 00 00 00 00 fa fa fa fa fa fa fa fa fa fa
[task 2017-09-07T10:06:20.791480Z] 10:06:20     INFO - GECKO(1101) |   0x0c0e80007f60: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
[task 2017-09-07T10:06:20.793357Z] 10:06:20     INFO - GECKO(1101) | Shadow byte legend (one shadow byte represents 8 application bytes):
[task 2017-09-07T10:06:20.795068Z] 10:06:20     INFO - GECKO(1101) |   Addressable:           00
[task 2017-09-07T10:06:20.796872Z] 10:06:20     INFO - GECKO(1101) |   Partially addressable: 01 02 03 04 05 06 07
[task 2017-09-07T10:06:20.802958Z] 10:06:20     INFO - GECKO(1101) |   Heap left redzone:       fa
[task 2017-09-07T10:06:20.804731Z] 10:06:20     INFO - GECKO(1101) |   Heap right redzone:      fb
[task 2017-09-07T10:06:20.806508Z] 10:06:20     INFO - GECKO(1101) |   Freed heap region:       fd
[task 2017-09-07T10:06:20.808306Z] 10:06:20     INFO - GECKO(1101) |   Stack left redzone:      f1
[task 2017-09-07T10:06:20.810269Z] 10:06:20     INFO - GECKO(1101) |   Stack mid redzone:       f2
[task 2017-09-07T10:06:20.812075Z] 10:06:20     INFO - GECKO(1101) |   Stack right redzone:     f3
[task 2017-09-07T10:06:20.813875Z] 10:06:20     INFO - GECKO(1101) |   Stack partial redzone:   f4
[task 2017-09-07T10:06:20.815577Z] 10:06:20     INFO - GECKO(1101) |   Stack after return:      f5
[task 2017-09-07T10:06:20.817321Z] 10:06:20     INFO - GECKO(1101) |   Stack use after scope:   f8
[task 2017-09-07T10:06:20.819074Z] 10:06:20     INFO - GECKO(1101) |   Global redzone:          f9
[task 2017-09-07T10:06:20.820787Z] 10:06:20     INFO - GECKO(1101) |   Global init order:       f6
[task 2017-09-07T10:06:20.822474Z] 10:06:20     INFO - GECKO(1101) |   Poisoned by user:        f7
[task 2017-09-07T10:06:20.824198Z] 10:06:20     INFO - GECKO(1101) |   Container overflow:      fc
[task 2017-09-07T10:06:20.826448Z] 10:06:20     INFO - GECKO(1101) |   Array cookie:            ac
[task 2017-09-07T10:06:20.828330Z] 10:06:20     INFO - GECKO(1101) |   Intra object redzone:    bb
[task 2017-09-07T10:06:20.830305Z] 10:06:20     INFO - GECKO(1101) |   ASan internal:           fe
[task 2017-09-07T10:06:20.832117Z] 10:06:20     INFO - GECKO(1101) |   Left alloca redzone:     ca
[task 2017-09-07T10:06:20.833931Z] 10:06:20     INFO - GECKO(1101) |   Right alloca redzone:    cb
[task 2017-09-07T10:06:20.836055Z] 10:06:20     INFO - GECKO(1101) | ==1101==ABORTING
[task 2017-09-07T10:06:21.021119Z] 10:06:21     INFO - GECKO(1101) | ASAN:DEADLYSIGNAL
UAF related to SessionStorageManager. Baku, can you take a look? Thanks.
Group: core-security → dom-core-security
Component: XPCOM → DOM
Flags: needinfo?(amarchesini)
Attached patch storage.patchSplinter Review
Hard to say why this is happening. I suspect it's because LocalStorageManager and SessionStorageManager are both registered to the observer as weak references. LocalStorageManager triggers something that delete the docShell and SessionStorageManager is freed before receiving the notification from the observer.
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
Attachment #8905807 - Flags: review?(continuation)
Attachment #8905807 - Flags: review?(continuation) → review+
Comment on attachment 8905807 [details] [diff] [review]
storage.patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

This is a simple UAF crash.

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

No. I just wrote a comment about the using of nsTObserverArray instead of nsTArray in order to support the removal of elements when iterating.

Which older supported branches are affected by this flaw?

definitely beta. No ESR52.

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

bug 1333981 introduces StorageNotifierService.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

Easy to do in case this patch doesn't apply.

How likely is this patch to cause regressions; how much testing does it need?

none.
Attachment #8905807 - Flags: sec-approval?
Marking 55 as unaffected based on the July 6 checkin date for bug 1333981.

Sec-approval+ for trunk.
We'll want a beta patch made and nominated.
Attachment #8905807 - Flags: sec-approval? → sec-approval+
https://hg.mozilla.org/integration/mozilla-inbound/rev/5d74712bb1627824ef2abbc8fc4a5e28352e5f42

This grafts cleanly to Beta. Please request approval when you get a chance.
Flags: needinfo?(amarchesini)
Comment on attachment 8905807 [details] [diff] [review]
storage.patch

Approval Request Comment
[Feature/Bug causing the regression]: Bug1333981 and bug 1379568
[User impact if declined]: a crash can occur: UAF.
[Is this code covered by automated tests?]: no, this issue is a race condition
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: no 
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: none
[Why is the change risky/not risky?]: Instead using a normal nsTArray, we use a nsTObserverArray to guarantee that elements from the array can be removed when iterating.
[String changes made/needed]: none

This patch applies correctly to m-b.
Flags: needinfo?(amarchesini)
Attachment #8905807 - Flags: approval-mozilla-beta?
https://hg.mozilla.org/mozilla-central/rev/5d74712bb162
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Group: dom-core-security → core-security-release
Comment on attachment 8905807 [details] [diff] [review]
storage.patch

sec-high, uaf, beta56+
Attachment #8905807 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Group: core-security-release
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.