Console output: $ cat 2interesting/1testcase.js __defineSetter__("x", eval) __defineSetter__("a", /a/) o = (function () { for (l in [0, 0, 0]) { for (var [, x] = /x/ in this) print(x) } })() $ jsfunfuzz-dbg-64-tm-40659-4932aaad4962/js-dbg-64-tm-darwin -j -e maxRunTime=5000 2interesting/1testcase.js undefined undefined undefined undefined a undefined undefined undefined Assertion failed: LIR type error (start of writer pipeline): arg 1 of 'orq' is 'imml' which has type int32 (expected int64): 0 (../nanojit/LIR.cpp:2634) Abort trap asserts js debug shell on TM tip with -j and with "-e maxRunTime=5000" at Assertion failed: LIR type error (start of writer pipeline): arg 1 of 'orq' is 'imml' which has type int32 (expected int64): 0 (../nanojit/LIR.cpp:2634) I wonder why it requires "-e maxRunTime=5000", though the value "5000" can be any numerical value that I've tested. autoBisect shows this is probably related to bug 547314: The first bad revision is: changeset: 38505:65eeef03da7c user: Andreas Gal date: Mon Feb 22 16:30:22 2010 -0800 summary: Introduce ObjectOps for typeOf and make trace a mandatory ObjectOp (547314, r=brendan).

And oh, this seems 64-bit only, on both Mac and Linux.

Standalone testcase (only requires -j; thanks to Jesse for this tip): maxRunTime=0 __defineSetter__("x", eval) __defineSetter__("a", /a/) o = (function () { for (l in [0, 0, 0]) { for (var [, x] = /x/ in this) print(x) } })()

undefined undefined undefined undefined a undefined undefined undefined Assertion failed: LIR type error (start of writer pipeline): arg 1 of 'orq' is 'immi' which has type int32 (expected int64): 0 (../nanojit/LIR.cpp:2635) Program received signal SIGABRT, Aborted. 0x00007ffff6ebfa75 in *__GI_raise (sig=<value optimized out>) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 64 ../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory. in ../nptl/sysdeps/unix/sysv/linux/raise.c (gdb) bt #0 0x00007ffff6ebfa75 in *__GI_raise (sig=<value optimized out>) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 #1 0x00007ffff6ec35c0 in *__GI_abort () at abort.c:92 #2 0x000000000059f8e2 in avmplus::AvmAssertFail () at ../nanojit/avmplus.cpp:65 #3 0x000000000059b086 in nanojit::ValidateWriter::typeCheckArgs (this=0x8c3598, op=nanojit::LIR_orq, nArgs=2, formals=0x7fffffffd690, args=0x7fffffffd680) at ../nanojit/LIR.cpp:2630 #4 0x000000000059bbe1 in nanojit::ValidateWriter::ins2 (this=0x8c3598, op=nanojit::LIR_orq, a=0x8bece0, b=0x8beb28) at ../nanojit/LIR.cpp:2960 #5 0x000000000055d450 in js::TraceRecorder::box_jsval (this=0x8bdcc0, v=8769092, v_ins=0x8bece0) at ../jstracer.cpp:9396 #6 0x0000000000564115 in js::TraceRecorder::callNative (this=0x8bdcc0, argc=1, mode=JSOP_CALL) at ../jstracer.cpp:10891 #7 0x0000000000564ba6 in js::TraceRecorder::functionCall (this=0x8bdcc0, argc=1, mode=JSOP_CALL) at ../jstracer.cpp:11034 #8 0x000000000056b9e2 in js::TraceRecorder::record_JSOP_CALL (this=0x8bdcc0) at ../jstracer.cpp:12576 #9 0x0000000000554c8f in js::TraceRecorder::monitorRecording (this=0x8bdcc0, op=JSOP_CALL) at ../jsopcode.tbl:179 #10 0x00000000005b17ed in js_Interpret (cx=0x8a53f0) at ../jsops.cpp:78 #11 0x0000000000485ca2 in js_Execute (cx=0x8a53f0, chain=0x7ffff6c02000, script=0x8af750, down=0x0, flags=0, result=0x0) at ../jsinterp.cpp:1103 #12 0x000000000042598b in JS_ExecuteScript (cx=0x8a53f0, obj=0x7ffff6c02000, script=0x8af750, rval=0x0) at ../jsapi.cpp:4827 #13 0x0000000000403970 in Process (cx=0x8a53f0, obj=0x7ffff6c02000, filename=0x7fffffffe620 "1testcase.js", forceTTY=0) at ../../shell/js.cpp:449 #14 0x0000000000404786 in ProcessArgs (cx=0x8a53f0, obj=0x7ffff6c02000, argv=0x7fffffffe320, argc=2) at ../../shell/js.cpp:863 #15 0x000000000040c783 in main (argc=2, argv=0x7fffffffe320, envp=0x7fffffffe338) at ../../shell/js.cpp:5038 (gdb)

Kinda scary. I think bisecting might be pointing to a red herring.

Note that the original assertion was: Assertion failed: LIR type error (start of writer pipeline): arg 1 of 'orq' is 'imml' which has type int32 (expected int64): 0 (../nanojit/LIR.cpp and now it's morphed to: Assertion failed: LIR type error (start of writer pipeline): arg 1 of 'orq' is 'immi' which has type int32 (expected int64): 0 (../nanojit/LIR.cpp:2635) -> 'imml' changed to 'immi' Here's another testcase that shows the latter assertion: x = this for (let z = 0; z < 3; z++) { for (var [c] = 3 in x) { print(c) } }

(In reply to comment #5) > > -> 'imml' changed to 'immi' That opcode changed name very recently :) Bug 555633. So it hasn't really changed. LIR type errors are really bad, this needs to be fixed. It's Saturday morning in Melbourne so I don't have time to look in detail, but I suggest someone looks in TraceRecorder::box_jsval(), which has the only occurrences of LIR_pior in jstracer.cpp. (Nb: LIR_pior ends up as LIR_orq on 64-bit, it's a bit confusing because we're halfway through the opcode renaming.) Something that can help with diagnosing type errors is to change the NanoAssertMsgf() call in ValidateWriter::typeCheckArgs to a printf(); that way it doesn't stop on the error. Combine that with TMFLAGS=readlir and it shouldn't be too hard to find the bad LIR instruction.

> Something that can help with diagnosing type errors is to change the > NanoAssertMsgf() call in ValidateWriter::typeCheckArgs to a printf(); that way > it doesn't stop on the error. Combine that with TMFLAGS=readlir and it > shouldn't be too hard to find the bad LIR instruction. Area of code near where orq is located: 31: js_String_getelem1 = callq. #js_String_getelem ( cx $stack8 1 ) 32: eqq4 = eqq js_String_getelem1, NULL 33: xt2: xt eqq4 -> pc=0x1a4a7a4 imacpc=(nil) sp+56 rp+0 (GuardID=003) 34: stq.s sp[40] = js_String_getelem1 35: obj = immq 7F77:96E02000 36: stq.s sp[48] = obj 37: ATOM_TO_STRING(atom) = immq 0:85D120 38: stq.s sp[56] = ATOM_TO_STRING(atom) 39: JSVAL_STRING = immq 0:4 40: orq2 = orq js_String_getelem1, JSVAL_STRING

(In reply to comment #6) > LIR type errors are really bad, this needs to be fixed. Setting s-s first just to be safe.

The problem is in instruction 66 (marked with '*') in this attachment: * 66: orq1 = orq JSVAL_TO_SPECIAL(JSVAL_VOID), JSVAL_STRING JSVAL_TO_SPECIAL(JSVAL_VOID) is a 32-bit immediate. It must come from one of the INS_VOID() calls in jstracer.cpp. I'm pretty sure the 'orq' is generated by this line at the end of box_jsval(): return lir->ins2(LIR_pior, v_ins, INS_CONSTWORD(JSVAL_STRING)); I don't know much beyond that, as I don't understand what this code is doing... maybe we need a pointer-sized alternative to INS_VOID()?

Explanation and whatnot coming in bug 546611, of which this is a dupe. comment #5 and comment #10 show this is clearly and easily exploitable. Once you can tag an integer as an object/string, game over.

dvander, can you CC me on bug 546611? Thanks.

qawanted: Is this really a dupe of bug 546611? Gary thought that one regressed Dec 11 due to bug 515749 and this one Feb 22 due to bug 547314. Gary: it would be great if you could verify that this bug is really fixed on trunk and 3.6.4

Dan's right; this isn't a dupe (I tested using the testcase in comment 2). Switching back to [sg:investigate]. I couldn't get the comment 2 testcase to assert in 1.9.2 shell builds though. Nor could I get the comment 5 testcase to assert in any of the shells. TM: $ ~/Desktop/jsfunfuzz-dbg-64-tm-41833-d9ef93881da0/js-dbg-64-tm-linux -j js> maxRunTime=0 0 js> __defineSetter__("x", eval) js> __defineSetter__("a", /a/) js> o = (function () { for (l in [0, 0, 0]) { for (var [, x] = /x/ in this) print(x) } })() undefined undefined undefined undefined a undefined undefined undefined Assertion failed: LIR type error (start of writer pipeline): arg 1 of 'orq' is 'immi' which has type int32 (expected int64): 0 (../nanojit/LIR.cpp:2640) Aborted m-c: $ ~/Desktop/jsfunfuzz-dbg-64-mc-42011-b6726b0fa230/js-dbg-64-mc-linux -j js> maxRunTime=0 0 js> __defineSetter__("x", eval) js> __defineSetter__("a", /a/) js> o = (function () { for (l in [0, 0, 0]) { for (var [, x] = /x/ in this) print(x) } })() undefined undefined undefined undefined a undefined undefined undefined Assertion failed: LIR type error (start of writer pipeline): arg 1 of 'orq' is 'immi' which has type int32 (expected int64): 0 (../nanojit/LIR.cpp:2640) Aborted

clearing the deprecated [sg:investigate] so it gets covered in the weekly mtg.

This looks like an assert failure in nanojit, which would suggest that this is not exploitable, unless this was seen in a debug build and asserts are no-ops in release builds of nonojit. Can someone shed some light on whether this is exploitable or not?

Nanojit assertions suggest that we're generating code wrong. I'll look into the test case in comment #15.

David, could you verify this fixes all tracing and JM bugs (if any)? Thanks, /be

Comment on attachment 446110 [details] [diff] [review] fix Grr, not right. /be

Just for posterity, here's the explanation of what's going wrong. o = (function () { for (l in [0, 0, 0]) { for (var [, x] = /x/ in this) print(x) } The destructuring assignment is not being recognized as a local slot, despite |var|. That's only half the bug. The real scary part (and why this asserts in the tracer) is that indirect eval calls js_GetCallObject for lightweight functions. This creates an intervening property on the call object, which the tracer does not see because of assumptions in BINDNAME.

The indirect eval is a global eval, but it gratuitously (left-over code) calls js_GetCallObject early on. David, Blake: can this be filed and fixed separately? It is not a security bug. /be

Okay, so just to make sure I'm understanding this correctly. The call object imports locals, and this script has a local "x" defined. But the wrong opcode is emitted, breaking assumptions in BINDNAME. The interpreter binds to the local slot, the tracer to the global. That's the security bug, so will file the the obj_eval one open. Thanks!

The indirect-eval-gets-Call-object bug is bug 566773. /be

http://hg.mozilla.org/tracemonkey/rev/0bed5abf1764 /be

Port to branch was pretty pedestrian, nothing tricky about it.

...likewise trivial.

Comment on attachment 505585 [details] [diff] [review] 191 patch Approved for 1.9.1.17, a=dveditz for release-drivers

Comment on attachment 505589 [details] [diff] [review] 192 patch Approved for 1.9.2.14, a=dveditz for release-drivers Needs to land by 13:00 PST, can you do that today?

Aye aye, cap'n! http://hg.mozilla.org/releases/mozilla-1.9.1/rev/6e7c740fdcb5 http://hg.mozilla.org/releases/mozilla-1.9.2/rev/7a28315a5aec

Based on comment 15, what is the best way to verify this fix for 1.9.1 and 1.9.2?

Tracer has been long gone on trunk, marking verified.

Filter on qa-project-auto-change: Bug in removed tracer code, setting in-testsuite- flag.