Closed Bug 1276013 Opened 8 years ago Closed 8 years ago

heap-use-after-free in nsWindowWatcher::OpenWindowInternal

Categories

(Core :: DOM: Content Processes, defect)

Product:
Core
Component:
DOM: Content Processes
Version:
49 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla49
Tracking Flags:
Tracking Status
e10s m9+ ---
firefox47 --- disabled
firefox48 + fixed
firefox49 + fixed
firefox-esr45 - disabled

People

(Reporter: nils, Assigned: smaug)

References

Details

(Keywords: csectype-uaf, regression, sec-high, Whiteboard: btpp-active, e10s-only)

Attachments

(2 files)

patch
1.08 KB, patch
mconley
: review+
dveditz
: approval-mozilla-aurora+
lizzard
: approval-mozilla-esr45-
dveditz
: sec-approval+
Details | Diff | Splinter Review
for aurora
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
Component: IPC → DOM: Content Processes
Keywords: csectype-uaf, sec-critical
Assignee: nobody → bugs
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.
[Tracking Requested - why for this release]: e10s-only sec-critical.
status-firefox47: --- → disabled
status-firefox48: --- → affected
status-firefox-esr45: --- → disabled
tracking-firefox48: --- → ?
tracking-firefox49: --- → ?
Attached patch patch — — Splinter Review 
Silly me. I didn't notice this in bug 1218594.
Attachment #8757016 - Flags: review?(mconley)
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)
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)
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 on attachment 8757016 [details] [diff] [review]
patch

Review of attachment 8757016 [details] [diff] [review]:
-----------------------------------------------------------------

Ouch. :( Today I learned.
Attachment #8757016 - Flags: review?(mconley) → review+
I highly suspect we'll want to track this for M9.
tracking-e10s: --- → m9+
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.
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.
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 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?
Filed bug 1276052 for the prefetch service. Filed bug 1276058 for some maybe static analysis to avoid this in the future.
Tracking for 48/49 since this is sec-critical.
tracking-firefox48: ? → +
tracking-firefox49: ? → +
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
If this winds up being a sec-low, then it doesn't need sec-approval+ to be checked into trunk.
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.
that sounds higher than sec-low.

Should this get sec-approval before landing?
Flags: needinfo?(dveditz)
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.
Keywords: sec-low → sec-high
Waiting for sec-approval on this before approving uplift requests.
Whiteboard: btpp-active
(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 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+
No advisory on this one, per discussion with Dan.
Never mind, Dan meant a different bug.
https://hg.mozilla.org/mozilla-central/rev/3d13e005cdc3
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox49: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
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)
Attached patch for aurora — — Splinter Review 
Flags: needinfo?(bugs)

    
    

    
  
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:
          --- → ?

        Attachment #8757016 -
        Flags: approval-mozilla-esr45? → approval-mozilla-esr45-
Group: dom-core-security → core-security-release
Flags: sec-bounty?
Flags: sec-bounty? → sec-bounty+
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)

    
    

    
  
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)

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: