Closed Bug 1060398 Opened 6 years ago Closed 6 years ago

Assertion failure: obj->as<ArrayObject>().lengthIsWritable(), at jit/VMFunctions.cpp:404

Categories

(Core :: JavaScript Engine: JIT, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla35
Tracking Status
firefox32 --- unaffected
firefox33 --- unaffected
firefox34 + verified
firefox35 + verified
firefox-esr31 --- unaffected
b2g-v2.0 --- unaffected
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed

People

(Reporter: decoder, Assigned: jandem)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, sec-moderate, testcase, Whiteboard: [jsbugmon:update][b2g-adv-main2.2-])

Attachments

(2 files)

The following testcase asserts on mozilla-central revision d697d649c765 (run with --no-threads --fuzzing-safe --ion-eager):


try {
  function f(arr)
  assertEq(arr.push(4), 5);
  function test(out) {
    var arrs = out.arrs = [];
    arrs.push([0, 1, 2, 3]);
    var a = [0, 1, 2, 3, 4, 5, 6, 7];
    Object.defineProperty(a, "length", { writable: false, value: 4 });
    arrs.push(a);
    for (var i = 0, sz = arrs.length; i < sz; i++) {
      var arr = arrs[i];
      f(arr);
    }
  }
  var obj = {};
  test(obj);
} catch(exc0) {}
test(1);
Marked s-s because I don't know what the implications of this assertion are. Length and writable sounds like potential trouble to me.
Whiteboard: [jsbugmon:update,bisect]
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/a4877238de09
user:        Jan de Mooij
date:        Thu Aug 21 18:51:40 2014 +0200
summary:     Bug 1056795 - Optimize ArrayPushDense. r=bhackett

This iteration took 327.179 seconds to run.
Needinfo from Jan based on comment 3.
Flags: needinfo?(jdemooij)
Attached patch PatchSplinter Review
Non-writable array length is not tracked in type information but is implemented by changing the capacity, so we still need to check for it.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Attachment #8482183 - Flags: review?(bhackett1024)
Flags: needinfo?(jdemooij)
Attachment #8482183 - Flags: review?(bhackett1024) → review+
Jandem, could this cause us to write out of bounds or something?  What are the security implications?
Flags: needinfo?(jdemooij)
(In reply to Andrew McCreight [:mccr8] from comment #6)
> Jandem, could this cause us to write out of bounds or something?  What are
> the security implications?

It's a correctness issue: we shouldn't be able to append values to an array with non-writable length. I don't think it has security implications, but I'll mark it sec-moderate for now. It's also a very simple patch that we should just backport to Aurora.
Flags: needinfo?(jdemooij)
Keywords: sec-moderate
Comment on attachment 8482183 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: Bug 1056795
[User impact if declined]: Broken websites, maybe more serious issues.
[Describe test coverage new/current, TBPL]: Tested on TBPL.
[Risks and why]: No/low risk.
[String/UUID change made/needed]: None.
Attachment #8482183 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/3b31478e8a85
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Blocks: 1056795
Comment on attachment 8482183 [details] [diff] [review]
Patch

Aurora+
Attachment #8482183 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
JSBugMon: This bug has been automatically verified fixed on Fx34
Group: core-security
Whiteboard: [jsbugmon:update] → [jsbugmon:update][b2g-adv-main2.2-]
You need to log in before you can comment on or make changes to this bug.