Assertion failure: rt->gc.atomMarking.atomIsMarked in Intl.cpp

RESOLVED FIXED in Firefox 54

Status

()

RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: anba, Assigned: anba)

Tracking

({csectype-uaf, sec-high})

Trunk
mozilla55
csectype-uaf, sec-high
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox-esr45 unaffected, firefox52 unaffected, firefox-esr52 unaffected, firefox53 unaffected, firefox54+ fixed, firefox55+ fixed)

Details

Attachments

(3 attachments)

(Assignee)

Description

2 years ago
Test case:
---
var tz = new Intl.DateTimeFormat(undefined, {timeZone: "America/Indianapolis"}).resolvedOptions().timeZone;
var tz2 = newGlobal().eval(`
    new Intl.DateTimeFormat(undefined, {timeZone: "America/Indianapolis"}).resolvedOptions().timeZone;
`);
print(tz===tz2);
---

Asserts with:
---
Assertion failure: rt->gc.atomMarking.atomIsMarked(compartment->zone(), thing), at /home/andre/git/mozilla-central/js/src/jscntxtinlines.h:102
---
Keywords: csectype-uaf, sec-high
Group: core-security → javascript-core-security
(Assignee)

Comment 1

2 years ago
Created attachment 8855325 [details] [diff] [review]
bug1353694.patch

Adds the missing call to cx->markAtom() and changes the functions to use Atoms to match bug 1350760.
Assignee: nobody → andrebargull
Status: NEW → ASSIGNED
Attachment #8855325 - Flags: review?(jcoppeard)
(Assignee)

Comment 2

2 years ago
Created attachment 8855327 [details] [diff] [review]
bug1353694-testcase.patch (Don't yet checkin)

And the test case from comment #0.
Attachment #8855327 - Flags: review?(jcoppeard)

Updated

2 years ago
Attachment #8855325 - Flags: review?(jcoppeard) → review+

Updated

2 years ago
Attachment #8855327 - Flags: review?(jcoppeard) → review+
(Assignee)

Updated

2 years ago
Attachment #8855327 - Attachment description: bug1353694-testcase.patch → bug1353694-testcase.patch (Don't yet checkin)
(Assignee)

Updated

2 years ago
Keywords: checkin-needed
Does this affect more than 55? If so, you need to fill out sec-approval before it can be checked in.
Flags: needinfo?(andrebargull)
Keywords: checkin-needed
(Assignee)

Comment 4

2 years ago
(In reply to Andrew McCreight [:mccr8] from comment #3)
> Does this affect more than 55? If so, you need to fill out sec-approval
> before it can be checked in.

Ups, forgot about that.
Flags: needinfo?(andrebargull)
(Assignee)

Comment 5

2 years ago
(In reply to Andrew McCreight [:mccr8] from comment #3)
> Does this affect more than 55?

It was introduced by bug 1324002, which is currently Aurora (mozilla 54). So I'll better also prepare a patch for back-porting to Aurora.
status-firefox52: --- → unaffected
status-firefox53: --- → unaffected
status-firefox54: --- → affected
status-firefox-esr45: --- → unaffected
status-firefox-esr52: --- → unaffected
(Assignee)

Comment 6

2 years ago
Created attachment 8857472 [details] [diff] [review]
bug1353694-aurora.patch

Same patch as bug1353694.patch, only rebased for Aurora.
Attachment #8857472 - Flags: review?(jcoppeard)
Comment on attachment 8857472 [details] [diff] [review]
bug1353694-aurora.patch

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

Great.  I don't think you need to request review again if it's just a rebase.
Attachment #8857472 - Flags: review?(jcoppeard) → review+
(Assignee)

Comment 8

2 years ago
Comment on attachment 8855325 [details] [diff] [review]
bug1353694.patch

[Security approval request comment]

How easily could an exploit be constructed based on the patch?
- Given that it is basically a one-line fix, it shouldn't be too hard to figure out what was wrong with the previous version. But I can't give an estimation how easily it can be exploited, because I don't know what happens when markAtom() isn't called on a JSAtom which is shared across multiple compartments.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
- No comments were added and the test case is in a separate patch.

Which older supported branches are affected by this flaw?
- Only affects Nightly and Aurora.

If not all supported branches, which bug introduced the flaw?
- bug 1324002

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
- Yes, backport for Aurora is available.

How likely is this patch to cause regressions; how much testing does it need?
- Calling markAtom() shouldn't cause any regressions. Apart from the provided test case, no further testing is needed.
Attachment #8855325 - Flags: sec-approval?
Comment on attachment 8855325 [details] [diff] [review]
bug1353694.patch

sec-approval+ for trunk. Please nominate for aurora as well so we can fix it in both affected places.
Attachment #8855325 - Flags: sec-approval? → sec-approval+
(Assignee)

Comment 10

2 years ago
Comment on attachment 8857472 [details] [diff] [review]
bug1353694-aurora.patch

Approval Request Comment
[Feature/Bug causing the regression]: bug 1324002
[User impact if declined]: UAF, sec-high
[Is this code covered by automated tests?]: Yes, test case is attached (in a separate patch).
[Has the fix been verified in Nightly?]: No, patch for Nightly not yet landed.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Just adds calls to mark JSAtom.
[String changes made/needed]: None.
Attachment #8857472 - Flags: approval-mozilla-aurora?
Comment on attachment 8857472 [details] [diff] [review]
bug1353694-aurora.patch

sec-high, aurora54+
Attachment #8857472 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 12

2 years ago
uplift
https://hg.mozilla.org/integration/mozilla-inbound/rev/c16077181be0
https://hg.mozilla.org/releases/mozilla-aurora/rev/8c3b90886a8c
status-firefox54: affected → fixed
tracking-firefox54: --- → ?
tracking-firefox55: --- → ?
Flags: in-testsuite?
Tracking 54/55+ since this is sec high. Looks as if the trunk patch still has to land.
tracking-firefox54: ? → +
tracking-firefox55: ? → +
https://hg.mozilla.org/mozilla-central/rev/c16077181be0
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Group: javascript-core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.