Closed Bug 781855 Opened 12 years ago Closed 12 years ago

Assertion failure: [infer failure] Missing type in object [0xf6c00180] length: int, at jsinfer.cpp:325 with rest parameter

Categories

(Core :: JavaScript Engine, defect)

x86
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla17
Tracking Status
firefox14 --- unaffected
firefox15 --- unaffected
firefox16 --- fixed
firefox17 --- verified
firefox-esr10 --- unaffected

People

(Reporter: decoder, Assigned: efaust)

References

Details

(5 keywords, Whiteboard: [jsbugmon:update][advisory-tracking-])

Attachments

(2 files)

The following test asserts on mozilla-central revision b5ae446888f5 (options -m -n -a):


var Constr = function( ... property)  {};
Constr.prototype = [];
var c = new Constr();
c.push(5);
gc();
function enterFunc() {}
evaluate('enterFunc (c.length);');


S-s due to infer failure, which can be security-critical.
Whiteboard: [jsbugmon:update] → [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:   97629:60fedf46af8e
user:        Eric Faust
date:        Mon Jun 25 17:32:34 2012 -0700
summary:     Bug 764148 - Stop incorrectly shadowing TI properties on sets if a setter would
Eric, mind taking a look at this?
With Brian's help, was able to determine that the fix to 764148 elided marking certain native properties as own, even when they should have been. Fix on try at https://tbpl.mozilla.org/?tree=Try&rev=586b50d80390
Provisionally calling this sec-critical (inference).
Keywords: sec-critical
Attached patch FixSplinter Review
Assignee: general → efaustbmo
Status: NEW → ASSIGNED
Attachment #654299 - Flags: review?(bhackett1024)
Attachment #654300 - Flags: review?(bhackett1024)
Comment on attachment 654299 [details] [diff] [review]
Fix

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

::: js/src/methodjit/PolyIC.cpp
@@ +273,5 @@
>          Label stubShapeJumpLabel = masm.label();
>  
>          pic.setPropLabels().setStubShapeJump(masm, start, stubShapeJumpLabel);
>  
> +        if (pic.typeMonitored || adding) {

The comment below this should be updated to explain why the type guard is necessary in the adding case (we need to make sure that the property is marked as 'own' for objects of different type but the same shape), and to clarify that the existing comment only applies to the pic.typeMonitored case.
Attachment #654299 - Flags: review?(bhackett1024) → review+
Comment on attachment 654300 [details] [diff] [review]
Ionmonkey changes

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

::: js/src/ion/IonCaches.cpp
@@ +445,5 @@
>      MacroAssembler masm;
>  
>      Label failures;
>  
> +    /* Guard the Type of the object */

s/Type/type/
Attachment #654300 - Flags: review?(bhackett1024) → review+
https://hg.mozilla.org/mozilla-central/rev/455ed4a415aa
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Comment on attachment 654299 [details] [diff] [review]
Fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 764148
User impact if declined: Potential security risk (Type Inference)
Testing completed (on m-c, etc.): Tested on test in bug.
Risk to taking this patch (and alternatives if risky): Mild. Changes the way inference marks own properties. Alternatives include backing out 764148.
String or UUID changes made by this patch: None.
Attachment #654299 - Flags: approval-mozilla-aurora?
Original bug landed to central June 26, so should be also affecting FF 16, I believe.
Comment on attachment 654299 [details] [diff] [review]
Fix

[Triage Comment]
Low risk sec-critical fix for a FF16 regression. Approving for FF16.
Attachment #654299 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 654299 [details] [diff] [review]
Fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 764148
User impact if declined: Sec-critical assertion failure
Testing completed (on m-c, etc.): Tested on patch in this bug, as well as bugs 785576, 785749, 785824, and 789835 as well as successful m-c landing.
Risk to taking this patch (and alternatives if risky): Low. Minor TI change.
String or UUID changes made by this patch: None.

To land as single commit with patch in 785576 applied.
Attachment #654299 - Flags: approval-mozilla-beta?
Attachment #654299 - Flags: approval-mozilla-beta?
Attachment #654299 - Flags: approval-mozilla-beta+
Attachment #654299 - Flags: approval-mozilla-aurora+
Whiteboard: [jsbugmon:update] → [jsbugmon:update][advisory-tracking-]
Group: core-security
Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.