1511376 - Firefox crashes during startup when executed from tempdir [@ mozilla::dom::ContentParent::LaunchSubprocess(mozilla::hal::ProcessPriority) ]

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect, P1)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I can always crash Firefox during startup when calling the following mach commands for firefox-ui-update tests:

> ./mach firefox-ui-update

The script itself creates a copy of the application and launches the tests from the binary located under the temporary directory.

The following crash can be observed:


Operating system: Mac OS X
                  10.14.0 18A391
CPU: amd64
     family 6 model 61 stepping 4
     4 CPUs

GPU: UNKNOWN

Crash reason:  EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
Crash address: 0x0
Process uptime: 1 seconds

Thread 0 (crashed)
 0  XUL!mozilla::dom::ContentParent::LaunchSubprocess(mozilla::hal::ProcessPriority) [ContentParent.cpp:a60b595747ade6cdad6b51906bc9a880f6276f19 : 2121 + 0x0]
    rax = 0x000000011dac2c08   rdx = 0x0000000000000000
    rcx = 0x000000010fcc07a0   rbx = 0x0000000116c6d5a8
    rsi = 0x00000000001ce000   rdi = 0x00007ffedff7495c
    rbp = 0x00007ffedff752c0   rsp = 0x00007ffedff749e0
     r8 = 0x000000000000002f    r9 = 0x000000011e8243f0
    r10 = 0x0000000000000000   r11 = 0x0000000002db650c
    r12 = 0x0000000116c6d5a0   r13 = 0x000000011e88e6b8
    r14 = 0x00007ffedff75048   r15 = 0x000000011e88e6e0
    rip = 0x000000011afd471a
    Found by: given as instruction pointer in context
 1  XUL!mozilla::dom::ContentParent::GetNewOrUsedBrowserProcess(mozilla::dom::Element*, nsTSubstring<char16_t> const&, mozilla::hal::ProcessPriority, mozilla::dom::ContentParent*, bool) [ContentParent.cpp:a60b595747ade6cdad6b51906bc9a880f6276f19 : 908 + 0xe]
    rbp = 0x00007ffedff75400   rsp = 0x00007ffedff752d0
    rip = 0x000000011afd5bd8
    Found by: previous frame's frame pointer
 2  XUL!mozilla::dom::ContentParent::CreateBrowser(mozilla::dom::TabContext const&, mozilla::dom::Element*, mozilla::dom::ContentParent*, mozilla::dom::TabParent*, unsigned long long) [ContentParent.cpp:a60b595747ade6cdad6b51906bc9a880f6276f19 : 1232 + 0x18]
    rbp = 0x00007ffedff75640   rsp = 0x00007ffedff75410
    rip = 0x000000011afd8b5c
    Found by: previous frame's frame pointer
 3  XUL!nsFrameLoader::TryRemoteBrowser() [nsFrameLoader.cpp:a60b595747ade6cdad6b51906bc9a880f6276f19 : 2588 + 0x16]
    rbp = 0x00007ffedff75830   rsp = 0x00007ffedff75650
    rip = 0x000000011989ff9a
    Found by: previous frame's frame pointer
 4  XUL!nsFrameLoader::ShowRemoteFrame(mozilla::gfx::IntSizeTyped<mozilla::ScreenPixel> const&, nsSubDocumentFrame*) [nsFrameLoader.cpp:a60b595747ade6cdad6b51906bc9a880f6276f19 : 836 + 0x8]
    rbp = 0x00007ffedff75890   rsp = 0x00007ffedff75840
    rip = 0x00000001198a08c1
    Found by: previous frame's frame pointer
 5  XUL!nsFrameLoader::Show(int, int, int, int, nsSubDocumentFrame*) [nsFrameLoader.cpp:a60b595747ade6cdad6b51906bc9a880f6276f19 : 694 + 0x8]
    rbp = 0x00007ffedff759e0   rsp = 0x00007ffedff758a0
    rip = 0x00000001198a2cca
    Found by: previous frame's frame pointer
 6  XUL!nsSubDocumentFrame::ShowViewer() [nsSubDocumentFrame.cpp:a60b595747ade6cdad6b51906bc9a880f6276f19 : 191 + 0x10]
    rbp = 0x00007ffedff75a20   rsp = 0x00007ffedff759f0
    rip = 0x000000011b72929e
    Found by: previous frame's frame pointer
 7  XUL!AsyncFrameInit::Run() [nsSubDocumentFrame.cpp:a60b595747ade6cdad6b51906bc9a880f6276f19 : 98 + 0x5]
    rbp = 0x00007ffedff75a50   rsp = 0x00007ffedff75a30
    rip = 0x000000011b758d6a
    Found by: previous frame's frame pointer
 8  XUL!nsContentUtils::RemoveScriptBlocker() [nsContentUtils.cpp:a60b595747ade6cdad6b51906bc9a880f6276f19 : 5672 + 0x9]
    rbp = 0x00007ffedff75a90   rsp = 0x00007ffedff75a60
    rip = 0x00000001196d55c4
    Found by: previous frame's frame pointer
 9  XUL!nsDocument::EndUpdate() [nsDocument.cpp:a60b595747ade6cdad6b51906bc9a880f6276f19 : 5113 + 0x5]
    rbp = 0x00007ffedff75ad0   rsp = 0x00007ffedff75aa0
    rip = 0x0000000119866324
    Found by: previous frame's frame pointer
10  XUL!nsINode::ReplaceOrInsertBefore(bool, nsINode*, nsINode*, mozilla::ErrorResult&) [mozAutoDocUpdate.h:a60b595747ade6cdad6b51906bc9a880f6276f19 : 37 + 0xc]
    rbp = 0x00007ffedff75d80   rsp = 0x00007ffedff75ae0
    rip = 0x00000001198b55f2
    Found by: previous frame's frame pointer
11  XUL!mozilla::dom::Node_Binding::appendChild(JSContext*, JS::Handle<JSObject*>, nsINode*, JSJitMethodCallArgs const&) [nsINode.h:a60b595747ade6cdad6b51906bc9a880f6276f19 : 1775 + 0xf]
    rbp = 0x00007ffedff75e10   rsp = 0x00007ffedff75d90
    rip = 0x0000000119bc74f2
    Found by: previous frame's frame pointer
12  XUL!bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) [BindingUtils.cpp:a60b595747ade6cdad6b51906bc9a880f6276f19 : 3376 + 0x2]
    rbp = 0x00007ffedff75eb0   rsp = 0x00007ffedff75e20
    rip = 0x000000011a4ed55b
    Found by: previous frame's frame pointer
13  XUL!js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) [Interpreter.cpp:a60b595747ade6cdad6b51906bc9a880f6276f19 : 468 + 0x6]
    rbp = 0x00007ffedff75f70   rsp = 0x00007ffedff75ec0
    rip = 0x000000011cbce3cb
    Found by: previous frame's frame pointer
14  XUL!js::ForwardingProxyHandler::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const [Interpreter.cpp:a60b595747ade6cdad6b51906bc9a880f6276f19 : 634 + 0x8]
    rbp = 0x00007ffedff76040   rsp = 0x00007ffedff75f80
    rip = 0x000000011cf95b9a
    Found by: previous frame's frame pointer
15  XUL!js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const [CrossCompartmentWrapper.cpp:a60b595747ade6cdad6b51906bc9a880f6276f19 : 355 + 0x13]
    rbp = 0x00007ffedff760a0   rsp = 0x00007ffedff76050
    rip = 0x000000011cf7f9ab
    Found by: previous frame's frame pointer
16  XUL!js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) [Proxy.cpp:a60b595747ade6cdad6b51906bc9a880f6276f19 : 560 + 0x15]
    rbp = 0x00007ffedff760f0   rsp = 0x00007ffedff760b0
    rip = 0x000000011cf8b087
    Found by: previous frame's frame pointer
17  XUL!js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) [Interpreter.cpp:a60b595747ade6cdad6b51906bc9a880f6276f19 : 535 + 0xe]
    rbp = 0x00007ffedff761b0   rsp = 0x00007ffedff76100
    rip = 0x000000011cbce959
    Found by: previous frame's frame pointer
18  XUL!Interpret(JSContext*, js::RunState&) [Interpreter.cpp:a60b595747ade6cdad6b51906bc9a880f6276f19 : 620 + 0x8]
    rbp = 0x00007ffedff76670   rsp = 0x00007ffedff761c0
    rip = 0x000000011cbc7537
    Found by: previous frame's frame pointer
19  XUL!js::RunScript(JSContext*, js::RunState&) [Interpreter.cpp:a60b595747ade6cdad6b51906bc9a880f6276f19 : 447 + 0xb]
    rbp = 0x00007ffedff76750   rsp = 0x00007ffedff76680
    rip = 0x000000011cbbbf29
    Found by: previous frame's frame pointer
20  XUL!js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) [Interpreter.cpp:a60b595747ade6cdad6b51906bc9a880f6276f19 : 587 + 0x8]
    rbp = 0x00007ffedff76810   rsp = 0x00007ffedff76760
    rip = 0x000000011cbce7cc
    Found by: previous frame's frame pointer

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I did a regression test and ended up with the following list of potential changes:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2872e7a3606d6108874930a1eb4062c74bad0e9e&tochange=c9f6d6242851d25e230bc6a81fb45bfc8f93e6c1

Could this crash be related to the sandbox changes from bug 1431441? I wonder because it only happens when Firefox is executed from the temporary location.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

The last working revision is https://hg.mozilla.org/mozilla-central/rev/bb935ef08190

So it's really a regression from bug 1431441.

Comment 3

[Haik Aftandilian [:haik]]

The problem here is that, as of bug 1431441, we depend on the path to the firefox binary including the string ".app/Contents/MacOS". Typically, this is true because the firefox binary resides at {Firefox,Nightly}.app/Contents/MacOS/firefox, but it's not guaranteed to be true. If it isn't true, nsMacUtilsImpl::GetAppPath() fails and we intentionally crash the browser.

The firefox-ui-update test copies and renames the .app dir to a temp filename with a .application.copy suffix such as /var/folders/..../tmpFkfszm.application.copy and hence we crash when running firefox out of that directory.

Previously, the code to get the app paths was run in the child process and worked differently so this wasn't a problem. If possible, we should support the .app dir being renamed.

We need to determine the .app path for sandboxing purposes. Specifically, so that we can whitelist the entire directory to allow content processes to read support files from the directory.

Comment 4

[Haik Aftandilian [:haik]]

I'm testing a fix that doesn't depend on the presence of .app in Firefox's path. This fixes the crash, but I am hitting failures with the firefox-ui-update test. Henrik, is firefox-ui-update expected to pass locally on a Mac running 10.14? (See attached error log.)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4f7972e2a3f30cda9269845280539a25be041afd

Comment 5

[Haik Aftandilian [:haik]]

Attachment: Errors from firefox-ui-update with proposed fix.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

No, the test is expected to fail at the moment because an element id seems to have changed. I might fix this most likely with bug 1511312. So as long as it doesn't crash anymore, it will be fine. Thanks!

Comment 7

[Haik Aftandilian [:haik]]

Attachment: Bug 1511376 - Firefox crashes during startup when executed from tempdir r?Alex_Gaynor

Change nsMacUtilsImpl::GetAppPath() to not depend on the app bundle ending in ".app".

Comment 8

[Haik Aftandilian [:haik]]

I think we could improve this code by just getting the NS_GRE_DIR directory from the directory service and moving up two directories. That should work for the parent process and the child process and would be reusing existing logic we already have rather than searching within the argv0. I didn't want to make that change now being so close to the 65 merge.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4021b5422562
Firefox crashes during startup when executed from tempdir r=Alex_Gaynor

Comment 10

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/4021b5422562