Closed Bug 886277 Opened 6 years ago Closed 6 years ago

Assertion failure: isObject(), at ./dist/include/js/Value.h:1068 or Crash [@ js::CloseIterator]

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla25
Tracking Status
firefox21 --- unaffected
firefox22 --- unaffected
firefox23 + fixed
firefox24 + fixed
firefox25 + fixed
firefox-esr17 --- unaffected
b2g18 --- unaffected

People

(Reporter: decoder, Assigned: jandem)

References

(Blocks 2 open bugs)

Details

(5 keywords, Whiteboard: [jsbugmon:update,bisect][adv-main23-])

Crash Data

Attachments

(2 files, 1 obsolete file)

The following testcase asserts on mozilla-central revision 76820c6dff7b (run with --fuzzing-safe --ion-eager):


function gen2() {
  try {
    yield 2;
  } finally {
    throw gen2;
  }
}
function gen3() {
  yield 3;
}
label2: 
  for (let i in gen2())
    for (let j in gen3())
      break label2;
Marked s-s because the crash is not an obvious null deref, although it's still below 0x1000. Also this doesn't reproduce on 32 bit.
Crash Signature: [@ js::CloseIterator]
Keywords: crash
Whiteboard: [jsbugmon:update,bisect]
Attached patch PatchSplinter Review
Fix baseline to skip try notes with lower stack depth, matches the interpreter and fixes the crash.
Assignee: general → jdemooij
Status: NEW → ASSIGNED
Attachment #767075 - Flags: review?
Attachment #767075 - Flags: review? → review?(kvijayan)
Attachment #767075 - Flags: review?(kvijayan) → review+
Attachment #766631 - Attachment is obsolete: true
jandem, is this always going to be a null deref, or could it be quite bad?
Flags: needinfo?(jdemooij)
(In reply to Andrew McCreight [:mccr8] from comment #5)
> jandem, is this always going to be a null deref, or could it be quite bad?

No it's not always null. I'm not sure if an attacker can control the exact value but let's assume they can.
Flags: needinfo?(jdemooij)
Keywords: sec-critical
Comment on attachment 767075 [details] [diff] [review]
Patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch? It's possible but not very easy.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? There's a comment but it's pretty easy to get the same info from the code.

Which older supported branches are affected by this flaw? Beta, aurora.

If not all supported branches, which bug introduced the flaw? Baseline Compiler landing.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Should apply (or very easy to backport).

How likely is this patch to cause regressions; how much testing does it need? Unlikely.
Attachment #767075 - Flags: sec-approval?
Comment on attachment 767075 [details] [diff] [review]
Patch

Sec-approval+ for trunk. Please prepare and nominate branch patches once this lands. I'd like to avoid shipping this bug in a normal release.
Attachment #767075 - Flags: sec-approval? → sec-approval+
Comment on attachment 767075 [details] [diff] [review]
Patch

[Approval Request Comment]
> Bug caused by (feature/regressing bug #):
Baseline Compiler (new in 23)

> User impact if declined:
Security bugs, crashes

> Testing completed (on m-c, etc.):
On m-i

> Risk to taking this patch (and alternatives if risky): 
Pretty low risk

> String or IDL/UUID changes made by this patch:
None
Attachment #767075 - Flags: approval-mozilla-beta?
Attachment #767075 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/e63574cbbc97
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Attachment #767075 - Flags: approval-mozilla-beta?
Attachment #767075 - Flags: approval-mozilla-beta+
Attachment #767075 - Flags: approval-mozilla-aurora?
Attachment #767075 - Flags: approval-mozilla-aurora+
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update,bisect][adv-main23-]
Group: core-security
You need to log in before you can comment on or make changes to this bug.