Closed
Bug 1276013
Opened 8 years ago
Closed 8 years ago
heap-use-after-free in nsWindowWatcher::OpenWindowInternal
Categories
(Core :: DOM: Content Processes, defect)
Tracking
()
People
(Reporter: nils, Assigned: smaug)
References
Details
(Keywords: csectype-uaf, regression, sec-high, Whiteboard: btpp-active, e10s-only)
Attachments
(2 files)
1.08 KB,
patch
|
mconley
:
review+
dveditz
:
approval-mozilla-aurora+
lizzard
:
approval-mozilla-esr45-
dveditz
:
sec-approval+
|
Details | Diff | Splinter Review |
1.37 KB,
patch
|
Details | Diff | Splinter Review |
The following testcase crashes the latest ASAN build of Firefox (buildId 20160523171639) with E10s enabled. Testcase: <script> function start() { s= "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAundefinedAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAundefinedundefinedundefined"; try{document.open(undefined,s,undefined)}catch(e){};undefined; } </script> <body><a onclick="start()">Click Me (crashes the browser)</a></body> ASAN crash: ================================================================= ==14376==ERROR: AddressSanitizer: heap-use-after-free on address 0x6120003589c8 at pc 0x460a8e bp 0x7ffccd037db0 sp 0x7ffccd037d90 READ of size 133 at 0x6120003589c8 thread T0 #0 0x460a8d in __interceptor_strlen /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:509 #1 0x7fc116583f28 in length /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/string/nsCharTraits.h:498 #2 0x7fc116583f28 in nsDependentCString /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/string/nsTDependentString.h:54 #3 0x7fc116583f28 in AppendUTF8toUTF16(char const*, nsAString_internal&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/string/nsReadableUtils.cpp:269 #4 0x7fc11e14a413 in nsWindowWatcher::OpenWindowInternal(mozIDOMWindowProxy*, char const*, char const*, char const*, bool, bool, bool, nsITabParent*, nsIArray*, float*, mozIDOMWindowProxy**) /builds/slave/m-cen-l64-asan-000000000000000/build/src/embedding/components/windowwatcher/nsWindowWatcher.cpp:548 #5 0x7fc11e15040c in OpenWindow2 /builds/slave/m-cen-l64-asan-000000000000000/build/src/embedding/components/windowwatcher/nsWindowWatcher.cpp:449 #6 0x7fc11e15040c in non-virtual thunk to nsWindowWatcher::OpenWindow2(mozIDOMWindowProxy*, char const*, char const*, char const*, bool, bool, bool, nsITabParent*, nsISupports*, float, unsigned char, mozIDOMWindowProxy**) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/embedding/components/windowwatcher/Unified_cpp_windowwatcher0.cpp:454 #7 0x7fc11c468304 in mozilla::dom::ContentParent::RecvCreateWindow(mozilla::dom::PBrowserParent*, mozilla::dom::PBrowserParent*, mozilla::layout::PRenderFrameParent*, unsigned int const&, bool const&, bool const&, bool const&, nsString const&, nsCString const&, nsCString const&, mozilla::DocShellOriginAttributes const&, float const&, nsresult*, bool*, nsTArray<mozilla::dom::FrameScriptInfo>*, nsCString*, mozilla::layers::TextureFactoryIdentifier*, unsigned long*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/ipc/ContentParent.cpp:5542 #8 0x7fc117d8dc00 in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&, IPC::Message*&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/ipc/ipdl/PContentParent.cpp:8471 #9 0x7fc11746b417 in mozilla::ipc::MessageChannel::DispatchSyncMessage(IPC::Message const&, IPC::Message*&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1626 #10 0x7fc11746872b in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1589 #11 0x7fc117455fd2 in mozilla::ipc::MessageChannel::OnMaybeDequeueOne() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1560 #12 0x7fc11747b8d0 in applyImpl<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)()> /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:707 #13 0x7fc11747b8d0 in apply<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)()> /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:713 #14 0x7fc11747b8d0 in nsRunnableMethodImpl<bool (mozilla::ipc::MessageChannel::*)(), false, true>::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:741 #15 0x7fc11747c39f in Run /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/ipc/MessageChannel.h:477 #16 0x7fc11747c39f in mozilla::ipc::MessageChannel::DequeueTask::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/ipc/MessageChannel.h:496 #17 0x7fc1166e98db in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/threads/nsThread.cpp:1073 #18 0x7fc116763e2a in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:290 #19 0x7fc11747356e in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessagePump.cpp:98 #20 0x7fc1173e822c in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:235 #21 0x7fc1173e822c in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:228 #22 0x7fc1173e822c in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:208 #23 0x7fc11cafc447 in nsBaseAppShell::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/widget/nsBaseAppShell.cpp:156 #24 0x7fc11ea0ad98 in nsAppStartup::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/components/startup/nsAppStartup.cpp:284 #25 0x7fc11eb0fb94 in XREMain::XRE_mainRun() /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4369 #26 0x7fc11eb10f88 in XREMain::XRE_main(int, char**, nsXREAppData const*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4473 #27 0x7fc11eb11e6e in XRE_main /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4581 #28 0x48a9a7 in do_main /builds/slave/m-cen-l64-asan-000000000000000/build/src/browser/app/nsBrowserApp.cpp:242 #29 0x48a9a7 in main /builds/slave/m-cen-l64-asan-000000000000000/build/src/browser/app/nsBrowserApp.cpp:382 #30 0x7fc1330d782f in __libc_start_main /build/glibc-GKVZIf/glibc-2.23/csu/../csu/libc-start.c:291 #31 0x489c8c in _start (/home/nils/fuzzer3/firefox/firefox+0x489c8c) 0x6120003589c8 is located 8 bytes inside of 261-byte region [0x6120003589c0,0x612000358ac5) freed by thread T0 here: #0 0x4720a1 in __interceptor_free /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64 #1 0x7fc11c468152 in IsVoid /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsTSubstring.h:95 #2 0x7fc11c468152 in mozilla::dom::ContentParent::RecvCreateWindow(mozilla::dom::PBrowserParent*, mozilla::dom::PBrowserParent*, mozilla::layout::PRenderFrameParent*, unsigned int const&, bool const&, bool const&, bool const&, nsString const&, nsCString const&, nsCString const&, mozilla::DocShellOriginAttributes const&, float const&, nsresult*, bool*, nsTArray<mozilla::dom::FrameScriptInfo>*, nsCString*, mozilla::layers::TextureFactoryIdentifier*, unsigned long*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/ipc/ContentParent.cpp:5539 #3 0x7fc117d8dc00 in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&, IPC::Message*&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/ipc/ipdl/PContentParent.cpp:8471 #4 0x7fc11746b417 in mozilla::ipc::MessageChannel::DispatchSyncMessage(IPC::Message const&, IPC::Message*&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1626 #5 0x7fc11746872b in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1589 #6 0x7fc117455fd2 in mozilla::ipc::MessageChannel::OnMaybeDequeueOne() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1560 #7 0x7fc11747b8d0 in applyImpl<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)()> /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:707 #8 0x7fc11747b8d0 in apply<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)()> /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:713 #9 0x7fc11747b8d0 in nsRunnableMethodImpl<bool (mozilla::ipc::MessageChannel::*)(), false, true>::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:741 #10 0x7fc11747c39f in Run /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/ipc/MessageChannel.h:477 #11 0x7fc11747c39f in mozilla::ipc::MessageChannel::DequeueTask::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/ipc/MessageChannel.h:496 #12 0x7fc1166e98db in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/threads/nsThread.cpp:1073 #13 0x7fc116763e2a in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:290 #14 0x7fc11747356e in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessagePump.cpp:98 #15 0x7fc1173e822c in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:235 #16 0x7fc1173e822c in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:228 #17 0x7fc1173e822c in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:208 #18 0x7fc11cafc447 in nsBaseAppShell::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/widget/nsBaseAppShell.cpp:156 #19 0x7fc11ea0ad98 in nsAppStartup::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/components/startup/nsAppStartup.cpp:284 #20 0x7fc11eb0fb94 in XREMain::XRE_mainRun() /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4369 #21 0x7fc11eb10f88 in XREMain::XRE_main(int, char**, nsXREAppData const*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4473 #22 0x7fc11eb11e6e in XRE_main /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4581 #23 0x48a9a7 in do_main /builds/slave/m-cen-l64-asan-000000000000000/build/src/browser/app/nsBrowserApp.cpp:242 #24 0x48a9a7 in main /builds/slave/m-cen-l64-asan-000000000000000/build/src/browser/app/nsBrowserApp.cpp:382 #25 0x7fc1330d782f in __libc_start_main /build/glibc-GKVZIf/glibc-2.23/csu/../csu/libc-start.c:291 previously allocated by thread T0 here: #0 0x4722a1 in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:74 #1 0x7fc116595da4 in Alloc /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/string/nsSubstring.cpp:217 #2 0x7fc116595da4 in nsACString_internal::MutatePrep(unsigned int, char**, unsigned int*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/string/nsTSubstring.cpp:133 #3 0x7fc1165a2b34 in nsACString_internal::SetCapacity(unsigned int, mozilla::fallible_t const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/string/nsTSubstring.cpp:671 #4 0x7fc11658340c in SetLength /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/string/nsTSubstring.cpp:709 #5 0x7fc11658340c in AppendUTF16toUTF8(nsAString_internal const&, nsACString_internal&, mozilla::fallible_t const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/string/nsReadableUtils.cpp:192 #6 0x7fc116584b38 in AppendUTF16toUTF8(nsAString_internal const&, nsACString_internal&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/string/nsReadableUtils.cpp:170 #7 0x7fc11c468139 in IsVoid /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsString.h:121 #8 0x7fc11c468139 in mozilla::dom::ContentParent::RecvCreateWindow(mozilla::dom::PBrowserParent*, mozilla::dom::PBrowserParent*, mozilla::layout::PRenderFrameParent*, unsigned int const&, bool const&, bool const&, bool const&, nsString const&, nsCString const&, nsCString const&, mozilla::DocShellOriginAttributes const&, float const&, nsresult*, bool*, nsTArray<mozilla::dom::FrameScriptInfo>*, nsCString*, mozilla::layers::TextureFactoryIdentifier*, unsigned long*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/ipc/ContentParent.cpp:5539 #9 0x7fc117d8dc00 in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&, IPC::Message*&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/ipc/ipdl/PContentParent.cpp:8471 #10 0x7fc11746b417 in mozilla::ipc::MessageChannel::DispatchSyncMessage(IPC::Message const&, IPC::Message*&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1626 #11 0x7fc11746872b in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1589 #12 0x7fc117455fd2 in mozilla::ipc::MessageChannel::OnMaybeDequeueOne() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1560 #13 0x7fc11747b8d0 in applyImpl<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)()> /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:707 #14 0x7fc11747b8d0 in apply<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)()> /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:713 #15 0x7fc11747b8d0 in nsRunnableMethodImpl<bool (mozilla::ipc::MessageChannel::*)(), false, true>::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/nsThreadUtils.h:741 #16 0x7fc11747c39f in Run /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/ipc/MessageChannel.h:477 #17 0x7fc11747c39f in mozilla::ipc::MessageChannel::DequeueTask::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/ipc/MessageChannel.h:496 #18 0x7fc1166e98db in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/threads/nsThread.cpp:1073 #19 0x7fc116763e2a in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:290 #20 0x7fc11747356e in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessagePump.cpp:98 #21 0x7fc1173e822c in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:235 #22 0x7fc1173e822c in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:228 #23 0x7fc1173e822c in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:208 #24 0x7fc11cafc447 in nsBaseAppShell::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/widget/nsBaseAppShell.cpp:156 #25 0x7fc11ea0ad98 in nsAppStartup::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/components/startup/nsAppStartup.cpp:284 #26 0x7fc11eb0fb94 in XREMain::XRE_mainRun() /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4369 #27 0x7fc11eb10f88 in XREMain::XRE_main(int, char**, nsXREAppData const*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4473 #28 0x7fc11eb11e6e in XRE_main /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsAppRunner.cpp:4581 #29 0x48a9a7 in do_main /builds/slave/m-cen-l64-asan-000000000000000/build/src/browser/app/nsBrowserApp.cpp:242 #30 0x48a9a7 in main /builds/slave/m-cen-l64-asan-000000000000000/build/src/browser/app/nsBrowserApp.cpp:382 #31 0x7fc1330d782f in __libc_start_main /build/glibc-GKVZIf/glibc-2.23/csu/../csu/libc-start.c:291 SUMMARY: AddressSanitizer: heap-use-after-free /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:509 __interceptor_strlen Shadow bytes around the buggy address: 0x0c24800630e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c24800630f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c2480063100: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c2480063110: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c2480063120: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa =>0x0c2480063130: fa fa fa fa fa fa fa fa fd[fd]fd fd fd fd fd fd 0x0c2480063140: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2480063150: fd fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa 0x0c2480063160: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00 0x0c2480063170: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c2480063180: 00 00 00 00 00 00 00 00 00 00 02 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 Contiguous container OOB:fc ASan internal: fe ==14376==ABORTING
Updated•8 years ago
|
Component: IPC → DOM: Content Processes
Keywords: csectype-uaf,
sec-critical
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → bugs
Comment 1•8 years ago
|
||
Hmm the line numbers don't make a lot of sense, but it looks like the general gist of it is that RecvCreateWindow() isn't keeping some string alive properly.
Comment 2•8 years ago
|
||
[Tracking Requested - why for this release]: e10s-only sec-critical.
status-firefox47:
--- → disabled
status-firefox48:
--- → affected
status-firefox-esr45:
--- → disabled
tracking-firefox48:
--- → ?
tracking-firefox49:
--- → ?
Assignee | ||
Comment 3•8 years ago
|
||
Silly me. I didn't notice this in bug 1218594.
Attachment #8757016 -
Flags: review?(mconley)
Comment 4•8 years ago
|
||
Feeling like kind of a noob asking this - but how does this fix the crash? You're taking the same value that was being assigned to `name` and passing it into OpenWindow2... how was that getting cleared before, and how does your patch address it? I feel like I'm missing an important subtlety here.
Flags: needinfo?(bugs)
Assignee | ||
Comment 5•8 years ago
|
||
NS_ConvertUTF16toUTF8(aName) is creating an object which is deleted at the end of the expression. Meaning that after const char* name = aName.IsVoid() ? nullptr : NS_ConvertUTF16toUTF8(aName).get(); name points to a deleted area in the memory. It just happen to work usually. Creating the NS_ConvertUTF16toUTF8(aName) during the call-expression calls the dtor of that object once the call has been done.
Flags: needinfo?(bugs)
Assignee | ||
Comment 6•8 years ago
|
||
We have comment somewhere in the tree about this, or maybe in MDN, but can't find it now. There is at least http://stackoverflow.com/questions/3041959/life-scope-of-temporary-variable
Comment 7•8 years ago
|
||
Comment on attachment 8757016 [details] [diff] [review] patch Review of attachment 8757016 [details] [diff] [review]: ----------------------------------------------------------------- Ouch. :( Today I learned.
Attachment #8757016 -
Flags: review?(mconley) → review+
Comment 9•8 years ago
|
||
This looks like it would be pretty hard to exploit, as I'm not sure any user-controlled code runs in between the free and the use, but better safe than sorry.
Assignee | ||
Comment 10•8 years ago
|
||
Here http://mxr.mozilla.org/mozilla-central/source/xpcom/string/nsTPromiseFlatString.h#27
Comment 11•8 years ago
|
||
Hrm. Here's another bad use, I think: https://dxr.mozilla.org/mozilla-central/source/uriloader/prefetch/nsPrefetchService.cpp#827 Might be worth adding some static analysis for this.
Assignee | ||
Comment 12•8 years ago
|
||
Comment on attachment 8757016 [details] [diff] [review] patch [Security approval request comment] How easily could an exploit be constructed based on the patch? It is a bit hard to get user code to run in the parent process Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? Just the patch itself pinpoint the issue Which older supported branches are affected by this flaw? 45+. But this affect e10s/multiprocess setups only. If not all supported branches, which bug introduced the flaw? bug 1218594 Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? This should be very safe. Just using a temporary object safely How likely is this patch to cause regressions; how much testing does it need? should be very safe.
Attachment #8757016 -
Flags: sec-approval?
Attachment #8757016 -
Flags: approval-mozilla-esr45?
Attachment #8757016 -
Flags: approval-mozilla-beta?
Attachment #8757016 -
Flags: approval-mozilla-aurora?
Comment 13•8 years ago
|
||
Comment on attachment 8757016 [details] [diff] [review] patch The only build left in beta is a release candidate, so I think we shouldn't take this there, given that it only affects e10s.
Attachment #8757016 -
Flags: approval-mozilla-beta?
Comment 14•8 years ago
|
||
Filed bug 1276052 for the prefetch service. Filed bug 1276058 for some maybe static analysis to avoid this in the future.
Comment 16•8 years ago
|
||
Worst-case seems to be the memory got reallocated with attacker-supplied data, and then what? We target a different name than the original one? That already doesn't sound too bad, and especially when you consider that the original name was (most likely) given by the attacker in the first place. I'm lowering the security severity but please correct me if I'm wrong.
Group: core-security → dom-core-security
Keywords: sec-critical → sec-low
Comment 17•8 years ago
|
||
If this winds up being a sec-low, then it doesn't need sec-approval+ to be checked into trunk.
Reporter | ||
Comment 18•8 years ago
|
||
In the worst case the memory could get reallocated with data containing pointers or other sensitive information which could be read by an attacker through the "name" property.
Assignee | ||
Comment 19•8 years ago
|
||
that sounds higher than sec-low. Should this get sec-approval before landing?
Flags: needinfo?(dveditz)
Comment 20•8 years ago
|
||
I think this should be sec-high. NS_ConvertUTF16toUTF8 creates a string, which is then destroyed, and we pass in a dead pointer to OpenWindow2. Before using the dead pointer argument, this calls into ConvertArgsToArray, which I think allocates an array of user-controllable size (to an extent). If this nsIMutableArray is in the same size class as the string we allocated earlier, then jemalloc could reuse the same block, and content could potentially then read out pointer values from the string.
Waiting for sec-approval on this before approving uplift requests.
Updated•8 years ago
|
Whiteboard: btpp-active
Comment 22•8 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #19) > that sounds higher than sec-low. Should this get sec-approval before landing? Yes and yes.
Blocks: 1218594
Flags: needinfo?(dveditz)
Keywords: regression
Whiteboard: btpp-active → btpp-active, e10s-only
Comment 23•8 years ago
|
||
Comment on attachment 8757016 [details] [diff] [review] patch sec-approval=dveditz Since this doesn't affect Release/ESR (disabled) go ahead and land. a=dveditz for aurora (48) Do we want this on ESR-45? It's disabled there, are we worried corporate folks will be playing with e10s over the next year? Since the patch points out a bad pattern we need to audit for and an ESR landing is much higher profile than Aurora let's at least wait until the next cycle to land on ESR.
Attachment #8757016 -
Flags: sec-approval?
Attachment #8757016 -
Flags: sec-approval+
Attachment #8757016 -
Flags: approval-mozilla-aurora?
Attachment #8757016 -
Flags: approval-mozilla-aurora+
Comment 24•8 years ago
|
||
No advisory on this one, per discussion with Dan.
Comment 25•8 years ago
|
||
Never mind, Dan meant a different bug.
Assignee | ||
Comment 26•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d13e005cdc3a61a026db1811e2e9bec90229046
Comment 27•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3d13e005cdc3
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Comment 28•8 years ago
|
||
has problems uplifting to aurora: grafting 347745:3d13e005cdc3 "bug 1276013, fix regression in e10s window name handling, r=mconley" merging dom/ipc/ContentParent.cpp warning: conflicts while merging dom/ipc/ContentParent.cpp! (edit, then use 'hg resolve --mark') abort: unresolved conflicts, can't continue (use 'hg resolve' and 'hg graft --continue')
Flags: needinfo?(bugs)
Assignee | ||
Comment 29•8 years ago
|
||
Flags: needinfo?(bugs)
Comment 30•8 years ago
|
||
Adding esr45 tracking:? to ensure we come back to evaluate this for 45.3.0esr or beyond. But I will take off the approval request for now.
tracking-firefox-esr45:
--- → ?
Updated•8 years ago
|
Attachment #8757016 -
Flags: approval-mozilla-esr45? → approval-mozilla-esr45-
Updated•8 years ago
|
Group: dom-core-security → core-security-release
Updated•8 years ago
|
Flags: sec-bounty?
Updated•8 years ago
|
Flags: sec-bounty? → sec-bounty+
Updated•8 years ago
|
Group: core-security-release
Hello smaug, is this something we want to uplift to ESR45.3.0? If yes, please nominate a patch for uplift. Thanks!
Flags: needinfo?(bugs)
Assignee | ||
Comment 33•8 years ago
|
||
E10s isn't supported in 45, and certainly not enabled by default, so I think we don't need to take this.
Flags: needinfo?(bugs)
Updated•8 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•