Crash from launching Firefox on Mac with wrong capitalization
Categories
(Core :: Security: Process Sandboxing, defect)
Tracking
()
People
(Reporter: ray, Assigned: yanislgha, Mentored)
References
Details
(Keywords: good-first-bug, topcrash, topcrash-startup)
Crash Data
Attachments
(1 file)
Updated•3 years ago
|
Comment 2•1 year ago
|
||
no activity for a while, closing.
Comment 3•1 year ago
|
||
Please don't close bugs that have steps to reproduce without even trying them.
Comment 4•1 year ago
|
||
I tried running /Applications/Firefox\ Nightly.app/Contents/Macos/firefox -P test locally (note the Macos instead of MacOS) and the browser crashes immediately. Here's the crash report: bp-648731a6-391c-4044-961b-c1bd50240718
Comment 5•1 year ago
|
||
I'm on MacOS 14.5 (23F79).
Updated•1 year ago
|
Updated•1 year ago
|
Comment 6•1 year ago
•
|
||
This is seemingly due to our searching for "/Contents/MacOS" during startup: https://searchfox.org/mozilla-central/rev/91f6127b6f591da4037821791c345147d9a575da/xpcom/base/nsMacUtilsImpl.cpp#95
While this may not be the only issue that we might encounter, we should try changing this to a case insensitive search and see if anything else breaks.
| Assignee | ||
Comment 7•1 year ago
|
||
I'll try to warkaround this
| Assignee | ||
Comment 8•1 year ago
|
||
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Comment 10•1 year ago
|
||
| bugherder | ||
Comment 11•1 year ago
|
||
This causes crashes with the mentioned crash signature in the latest Nightly 131.0a1 20240810090653 which is the first build to have the patch included, e.g. bp-69fb3089-5c91-4708-a80f-d25b60240810. The change will be reverted.
Comment 12•1 year ago
|
||
Backed out for causing crashes in the latest nightly.
Comment 13•1 year ago
|
||
Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/d1e204ba2f75
| Assignee | ||
Comment 14•1 year ago
|
||
:aryx What’s causing crash ? In which platform ? I’ve tested it on macOS arm and x64 and it’s working perfectly
Comment 15•1 year ago
|
||
https://crash-stats.mozilla.org/report/index/ed6aec46-afb1-460f-bfd7-78d9d0240811 points to https://hg.mozilla.org/mozilla-central/file/7c116e941f9a22051b8714f871d3b9adcc298899/ipc/glue/GeckoChildProcessHost.cpp#l1898 as location triggering the crash. The app path is not found after the changes.
| Assignee | ||
Comment 16•1 year ago
|
||
Okay thanks I will check on that
| Assignee | ||
Comment 17•1 year ago
|
||
I can't reproduce the bug with my 2 macbooks both arm64 and amd64 so I will wait for my Access Level 1 to be granted in order to test it in real conditions. Let me know if anyone can guide me through the steps to reproduce the bug
Comment 18•1 year ago
|
||
I can push phabricator patches to Try if you want. Just let me know which patches and what tasks they shall run. Please be aware not every user experiences the crash. There are 67 crashes from 5-9 installs on macOS 12-15.
| Assignee | ||
Comment 19•1 year ago
|
||
That's why it's hard to me to find out the steps to reproduce, i've tested every path like even in an external storage, i've tested a lot of things but all works fine anytime, maybe send to try the actual patch i've made and see if any auto test fail. Also it's weird because it appear to be a random crash at all
Comment 20•1 year ago
|
||
One possible issue I see is that the appPath.Assign(Substring(start, end)) is now getting the lower case path string. Previously appPath was set to "Firefox.app" but now it's set to "firefox.app" , which is then passed to NS_NewLocalFile.
| Assignee | ||
Comment 21•1 year ago
|
||
Yeah that could definitively be that, I will take a look and see if it's true. But the side question is how could this append so randomly. Thanks for your help Mathew
Updated•1 year ago
|
Comment 22•1 year ago
|
||
The bug is linked to a topcrash signature, which matches the following criterion:
- Top 10 desktop browser crashes on nightly (startup)
:yanislgha, could you consider increasing the severity of this top-crash bug?
For more information, please visit BugBot documentation.
Comment 23•1 year ago
|
||
(In reply to BugBot [:suhaib / :marco/ :calixte] from comment #22)
The bug is linked to a topcrash signature, which matches the following criterion:
- Top 10 desktop browser crashes on nightly (startup)
:yanislgha, could you consider increasing the severity of this top-crash bug?
The landing of the patch in this bug most likely put this signature on the top-crash radar. Reverting the patch should bring the crash numbers back in line with what we were seeing before. The severity does not need to be adjusted.
Comment 24•1 year ago
•
|
||
I think the confusion here lies with the fact that, compared to the old crash, you would need the opposite configuration in terms of case sensitivity in order to reproduce the new crash.
On a case sensitive filesystem, macOS should directly refuse to start Firefox if given the capitalization found in comment 0. So I would say that the commit message is wrong -- the original crash can in fact only occur on case insensitive filesystems, which are the only ones on which Firefox can be started with this capitalization. As identified in comment 6, this original crash occurs because with that bad capitalization we can fail to catch the /Contents/MacOS substring.
As identified in comment 20, the new issue after the patch is a consequence of manipulating /applications/firefox nightly.app as the path to the application directory. However, that path will be accepted as correct on case insensitive filesystems, where the path will be treated as equivalent to /Applications/Firefox Nightly.app, so these systems will not encounter any trouble after the patch. On the other hand, users with a case sensitive filesystem will start to experience trouble, because this will not be accepted as a correct path for them.
So the patch appears to have converted a startup crash when using a wrong capitalization on case insensitive filesystems, into a startup crash when using the correct capitalization on case sensitive filesystems. You need the opposite setup to reproduce the new crash. The volume of the new crash was not dramatic because macOS creates case insensitive filesystems by default - so the majority of users cannot run into the new crash - but it was still rather high because this new crash can occur through the normal usage workflow of Firefox.
| Assignee | ||
Comment 25•1 year ago
|
||
(In reply to Yannis Juglaret [:yannis] from comment #24)
I think the confusion here lies with the fact that, compared to the old crash, you would need the opposite configuration in terms of case sensitivity in order to reproduce the new crash.
On a case sensitive filesystem, macOS should directly refuse to start Firefox if given the capitalization found in comment 0. So I would say that the commit message is wrong -- the original crash can in fact only occur on case insensitive filesystems, which are the only ones on which Firefox can be started with this capitalization. As identified in comment 6, this original crash occurs because with that bad capitalization we can fail to catch the
/Contents/MacOSsubstring.As identified in comment 20, the new issue after the patch is a consequence of manipulating
/applications/firefox nightly.appas the path to the application directory. However, that path will be accepted as correct on case insensitive filesystems, where the path will be treated as equivalent to/Applications/Firefox Nightly.app, so these systems will not encounter any trouble after the patch. On the other hand, users with a case sensitive filesystem will start to experience trouble, because this will not be accepted as a correct path for them.So the patch appears to have converted a startup crash when using a wrong capitalization on case insensitive filesystems, into a startup crash when using the correct capitalization on case sensitive filesystems. You need the opposite setup to reproduce the new crash. The volume of the new crash was not dramatic because macOS creates case insensitive filesystems by default - so the majority of users cannot run into the new crash - but it was still rather high because this new crash can occur through the normal usage workflow of Firefox.
Hi, thanks for your message, I didn't even know that macOS can switch to a case sentitive FS. I will modify the code to handle both. Thanks again
Updated•1 year ago
|
Updated•1 year ago
|
Comment 26•1 year ago
|
||
Comment 27•1 year ago
|
||
| bugherder | ||
Updated•1 year ago
|
Updated•1 year ago
|
Description
•