Closed Bug 1447156 Opened 2 years ago Closed 2 years ago

Crash in mozilla::dom::IDBFactory::OpenInternal

Categories

(Core :: Storage: IndexedDB, defect, P1, critical)

60 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 60+ fixed
firefox59 --- unaffected
firefox60 + fixed
firefox61 + fixed

People

(Reporter: philipp, Assigned: janv)

Details

(4 keywords)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is
report bp-2abc1d56-a0e3-4b3d-afd2-0e5b00180319.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll mozilla::dom::IDBFactory::OpenInternal dom/indexedDB/IDBFactory.cpp:716
1 xul.dll mozilla::dom::IDBFactory::Open dom/indexedDB/IDBFactory.cpp:504
2 xul.dll mozilla::dom::IDBFactoryBinding::open dom/bindings/IDBFactoryBinding.cpp:278
3 xul.dll mozilla::dom::GenericBindingMethod dom/bindings/BindingUtils.cpp:3031
4 xul.dll js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:468
5 xul.dll InternalCall js/src/vm/Interpreter.cpp:517
6 xul.dll Interpret js/src/vm/Interpreter.cpp:3086
7 xul.dll js::RunScript js/src/vm/Interpreter.cpp:418
8 xul.dll js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:490
9 xul.dll InternalCall js/src/vm/Interpreter.cpp:517

=============================================================

the signature for this browser crash during shutdown is regressing in volume in firefox 60.
Assignee: nobody → jvarga
Priority: -- → P1
:janv can you please investigate this regression once you have some time?
Flags: needinfo?(jvarga)
Emailed Jan.
Looking into this ...
Status: NEW → ASSIGNED
Any luck, Jan?
It seems that bug 1391865 could have regressed this, sMainThreadInfo is not nulled out when the thread local is destroyed.

However, I think it would be safer if we prevent actor re-create if the shutdown already started.

I'm working on a fix...
Flags: needinfo?(jvarga)
Attached patch PatchSplinter Review
This should fix the crash.
Attachment #8966555 - Flags: review?(bkelly)
Attachment #8966555 - Flags: review?(bkelly) → review+
Comment on attachment 8966555 [details] [diff] [review]
Patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Well, the first change in the patch nulls out a pointer which may point to a problem with "use after deletion".

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
No, there are no comments or tests. I'll also remove the bug summary from check-in comment which mentions "crash".

Which older supported branches are affected by this flaw?
Only 60 and 61 is affected.

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

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
It's easy to backport it.

How likely is this patch to cause regressions; how much testing does it need?
It was tested with a full try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=195802f8f2ab7c61a676f7e6dc171546862fc568
Attachment #8966555 - Flags: sec-approval?
Comment on attachment 8966555 [details] [diff] [review]
Patch

Sec-approval+ for trunk.
If this applies cleanly to Beta, please nominate the patch for Beta as well. Otherwise, we'll want a Beta patch made and nominated so we can avoid shipping this bug (and fix it before ESR60 is made).
Attachment #8966555 - Flags: sec-approval? → sec-approval+
https://hg.mozilla.org/mozilla-central/rev/42be71805c42
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Please request Beta approval on this when you're comfortable doing so.
Flags: needinfo?(jvarga)
Ok, it seems it's fixed on trunk, I'll request beta approval.
Group: dom-core-security → core-security-release
:janv did it get into beta?
Comment on attachment 8966555 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1391865
[User impact if declined]: Amount of crashes is not huge, but this is a sec-high bug.
[Is this code covered by automated tests?]: No, we only see this in crash reports.
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No, just crash stats verification.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No, very low risk.
[Why is the change risky/not risky?]: Changes are tiny and simple, there have been zero crashes in Nightly since April 7
[String changes made/needed]: None
Flags: needinfo?(jvarga)
Attachment #8966555 - Flags: approval-mozilla-beta?
Comment on attachment 8966555 [details] [diff] [review]
Patch

Low risk, sec-high, nightly data looks promising, Beta60+
Attachment #8966555 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
https://hg.mozilla.org/releases/mozilla-beta/rev/535bc3c0bb3e6218035e70ea03413f2fd7195938 (FIREFOX_60b_RELBRANCH)

ni?ryanvm for the m-r part.
Flags: needinfo?(ryanvm)
Flags: qe-verify-
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.