TM: "Assertion failure: !JSVAL_IS_PRIMITIVE(regs.sp[-2])" with __iterator__

VERIFIED FIXED

Status

()

Core
JavaScript Engine
P2
critical
VERIFIED FIXED
9 years ago
8 years ago

People

(Reporter: Jesse Ruderman, Assigned: mrbkap)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
x86
Mac OS X
assertion, crash, testcase, verified1.9.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +
wanted1.9.0.x -
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?] fixed-in-tracemonkey)

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

9 years ago
(function() {
  var a, b;
  for each (a in [{}, {__iterator__: function(){}}]) for (b in a) { }
})();

Assertion failure: !JSVAL_IS_PRIMITIVE(regs.sp[-2]), at ../jsinterp.cpp:3240

Gary Kwong found this with jsfunfuzz.
(Assignee)

Comment 1

9 years ago
This is because the for-(each-)in imacros don't vet the returned interator for primitiveness. The patch in bug 465460 adds a JSOP_IFPRIMTOP which makes this bug *much* nicer to fix, so I'll wait until that lands to write the patch.
Assignee: general → mrbkap
Depends on: 465460
This testcase causes a crash at js_CallIteratorNext near null in opt builds.

===

Exception Type:  EXC_BAD_ACCESS (SIGBUS)
Exception Codes: KERN_PROTECTION_FAILURE at 0x0000000000000014
Crashed Thread:  0

Thread 0 Crashed:
0   js-opt-tm-intelmac            	0x00048e94 js_CallIteratorNext + 18
1   js-opt-tm-intelmac            	0x000395f1 js_Interpret + 42321
2   js-opt-tm-intelmac            	0x00046bcb js_Execute + 559
3   js-opt-tm-intelmac            	0x000098ce JS_ExecuteScript + 60
4   js-opt-tm-intelmac            	0x00003100 Process(JSContext*, JSObject*, char*, int) + 1032
5   js-opt-tm-intelmac            	0x00006c68 main + 2310
6   js-opt-tm-intelmac            	0x0000254b _start + 209
7   js-opt-tm-intelmac            	0x00002479 start + 41
Keywords: crash
Flags: blocking1.9.1?

Updated

9 years ago
Flags: blocking1.9.1? → blocking1.9.1+

Comment 3

9 years ago
(In reply to comment #1)
> This is because the for-(each-)in imacros don't vet the returned interator for
> primitiveness. The patch in bug 465460 adds a JSOP_IFPRIMTOP which makes this
> bug *much* nicer to fix, so I'll wait until that lands to write the patch.

Bug 465460 is fixed now.

Updated

9 years ago
Priority: -- → P2
There's also a JSOP_PRIMTOP opcode which throws a TypeError if the top of the stack is primitive, but the error message is more generic than the current non--j error message.
eval("__proto__.__iterator__ = [].toString");
for (var z = 0; z < 3; ++z) { if (z % 3 == 2) { for(let y in []); } }

crashes opt TM at JS_GetMethodById at 0x20000004 and asserts debug TM identically. Security-sensitive now due to discovery of potentially exploitable testcase.

===

$ export TRACEMONKEY=verbose
$ ./js-dbg-tm-intelmac -j 2a.js 
recording starting from 2a.js:2@24
globalObj=0x294000, shape=147
    start
    state = param 0 ecx
    0
    sp = ld state[0]
    4
    rp = ld state[4]
    12
    cx = ld state[12]
    8
    gp = ld state[8]
    16
    eos = ld state[16]
    20
    eor = ld state[20]
    40
    globalObj = ld state[40]
    ld1 = ld cx[0]
    4096
    sub1 = sub ld1, 4096
    sti cx[0] = sub1
    le1 = le sub1, 0
    xt1: xt le1 -> 0:24 sp+0 rp+0

00024:   2  getgvar "z"
import vp=0x811738 name=$global0 type=int flags=0
00027:   2  int8 3
00029:   2  mod
    656
    ld2 = ld gp[656]
    $global0 = i2f ld2
    sti sp[0] = ld2
    #40080000:0 /* 3 */
    3
    sti sp[8] = 3
    js_imod1 = js_imod ( ld2 3 )

    -1
    eq1 = eq js_imod1, -1
    xt2: xt eq1 -> 0:29 sp+16 rp+0

00030:   2  int8 2
00032:   2  eq
    i2f1 = i2f js_imod1
    sti sp[0] = js_imod1
    #40000000:0 /* 2 */
    2
    sti sp[8] = 2
    eq2 = eq js_imod1, 2
    xt3: xt eq2 -> 0:32 sp+16 rp+0

00059:   2  incgvar "z"
    sti sp[0] = eq2
    #3FF00000:0 /* 1 */
    1
    add1 = add ld2, 1
    ov1 = ov add1
    xt4: xt ov1 -> 0:59 sp+0 rp+0

00062:   2  pop
00063:   2  getgvar "z"
00066:   2  int8 3
00068:   2  lt
    i2f2 = i2f add1
    sti sp[0] = add1
    st gp[656] = add1
    sti sp[0] = add1
    sti sp[8] = 3
    lt1 = lt add1, 3
    xf1: xf lt1 -> 0:68 sp+16 rp+0

Checking type stability against self=0x257004
global0 checkType(tag=1, t=1, isnum=1, i2f=1) stage_count=0
    sti sp[0] = lt1
    st gp[656] = add1
    loop

live instruction count 23, total 52, max pressure 5
side exits 1
state                                                        state = param 0 ecx
state sp                                                     sp = ld state[0]
state sp rp                                                  rp = ld state[4]
state sp cx                                                  cx = ld state[12]
state sp cx gp                                               gp = ld state[8]
state sp cx gp eos                                           eos = ld state[16]
state sp cx gp eor                                           eor = ld state[20]
state sp cx gp globalObj                                     globalObj = ld state[40]
state sp cx gp ld1                                           ld1 = ld cx[0]
state sp cx gp sub1                                          sub1 = sub ld1, 4096
state sp gp                                                  sti cx[0] = sub1
state sp gp ld2                                              ld2 = ld gp[656]
state sp gp ld2                                              sti sp[0] = ld2
state sp gp ld2                                              sti sp[8] = 3
state sp gp ld2 js_imod1                                     js_imod1 = js_imod ( ld2 3 )
state sp gp ld2                                              sti sp[0] = js_imod1
state sp gp ld2                                              sti sp[8] = 2
state sp gp add1                                             add1 = add ld2, 1
state sp gp add1                                             st gp[656] = add1
state sp gp add1                                             sti sp[0] = add1
state gp add1                                                sti sp[8] = 3
state                                                        st gp[656] = add1
state                                                        loop

    0x258f60  [prologue]                     
              nop9                           
              push edi                       
              push esi                       
              push ebx                       
              push ebp                       
              push ebp                       
              mov ebp,esp                    
    0x258f70  [frag entry]                   
              sub esp,8                      
patching 0x258ff3 to 0x258f73
        0x258f73:
compiling trunk 0x257004
    state = param 0 ecx
    sp = ld state[0]
    cx = ld state[12]
    gp = ld state[8]
    ld1 = ld cx[0]
    sub1 = sub ld1, 4096
    sti cx[0] = sub1
    le1 = le sub1, 0
    xt1: xt le1 -> 0:24 sp+0 rp+0
              mov -4(ebp),ecx                 ecx(state)
              mov ebx,ecx                     ecx(state)
              mov edi,0(ebx)                  ebx(state)
              mov eax,12(ebx)                 ebx(state) edi(sp)
              mov esi,8(ebx)                  eax(cx) ebx(state) edi(sp)
              mov ebx,0(eax)                  eax(cx) esi(gp) edi(sp)
              sub ebx,4096                    eax(cx) ebx(ld1) esi(gp) edi(sp)
              mov 0(eax),ebx                  eax(cx) ebx(sub1) esi(gp) edi(sp)
              cmp ebx,0                       ebx(sub1) esi(gp) edi(sp)
              jle 0x2a2fc1                    esi(gp) edi(sp)
--------------------------------------- exit block (LIR_xt|LIR_xf)
        0x2a2fc1:
                                              merging registers (intersect) with existing edge
              mov ecx,-4(ebp)                
              mov eax,2453748                
              mov esp,ebp                    
        0x2a2fcb:
              jmp 0x258ff8                   
--------------------------------------- end exit block 0x257138
    ld2 = ld gp[656]
    sti sp[0] = ld2
    sti sp[8] = 3
    js_imod1 = js_imod ( ld2 3 )
    eq1 = eq js_imod1, -1
    xt2: xt eq1 -> 0:29 sp+16 rp+0
              mov ebx,656(esi)                esi(gp) edi(sp)
              mov 0(edi),ebx                  ebx(ld2) esi(gp) edi(sp)
              mov 8(edi),3                    ebx(ld2) esi(gp) edi(sp)
              mov edx,3                       ebx(ld2) esi(gp) edi(sp)
              mov ecx,ebx                     ebx(ld2) esi(gp) edi(sp)
              call js_imod                    ebx(ld2) esi(gp) edi(sp)
              mov ecx,-4(ebp)                 ebx(ld2) esi(gp) edi(sp)
              cmp eax,-1                      eax(js_imod1) ecx(state) ebx(ld2) esi(gp) edi(sp)
              je 0x2a2fd0                     eax(js_imod1) ecx(state) ebx(ld2) esi(gp) edi(sp)
--------------------------------------- exit block (LIR_xt|LIR_xf)
        0x2a2fd0:
              mov eax,2453876                
              mov esp,ebp                    
        0x2a2fd7:
              jmp 0x258ff8                   
--------------------------------------- end exit block 0x2571b8
    sti sp[0] = js_imod1
    sti sp[8] = 2
    eq2 = eq js_imod1, 2
    xt3: xt eq2 -> 0:32 sp+16 rp+0
              mov 0(edi),eax                  eax(js_imod1) ecx(state) ebx(ld2) esi(gp) edi(sp)
              mov 8(edi),2                    eax(js_imod1) ecx(state) ebx(ld2) esi(gp) edi(sp)
              cmp eax,2                       eax(js_imod1) ecx(state) ebx(ld2) esi(gp) edi(sp)
              je 0x2a2fdc                     ecx(state) ebx(ld2) esi(gp) edi(sp)
--------------------------------------- exit block (LIR_xt|LIR_xf)
        0x2a2fdc:
              mov eax,2453980                
              mov esp,ebp                    
        0x2a2fe3:
              jmp 0x258ff8                   
--------------------------------------- end exit block 0x257220
    add1 = add ld2, 1
    ov1 = ov add1
    xt4: xt ov1 -> 0:59 sp+0 rp+0
              add ebx,1                       ecx(state) ebx(ld2) esi(gp) edi(sp)
              jo 0x2a2fe8                     ecx(state) ebx(add1) esi(gp) edi(sp)
--------------------------------------- exit block (LIR_xt|LIR_xf)
        0x2a2fe8:
              mov eax,2454076                
              mov esp,ebp                    
        0x2a2fef:
              jmp 0x258ff8                   
--------------------------------------- end exit block 0x257284
    st gp[656] = add1
    sti sp[0] = add1
    sti sp[8] = 3
    lt1 = lt add1, 3
    xf1: xf lt1 -> 0:68 sp+16 rp+0
              mov 656(esi),ebx                ecx(state) ebx(add1) esi(gp) edi(sp)
              mov 0(edi),ebx                  ecx(state) ebx(add1) esi(gp) edi(sp)
              mov 8(edi),3                    ecx(state) ebx(add1) esi(gp) edi(sp)
              cmp ebx,3                       ecx(state) ebx(add1) esi(gp)
              jnl 0x2a2ff4                    ecx(state) ebx(add1) esi(gp)
--------------------------------------- exit block (LIR_xt|LIR_xf)
        0x2a2ff4:
              mov eax,2454176                
              mov esp,ebp                    
        0x2a2ffb:
              jmp 0x258ff8                   
--------------------------------------- end exit block 0x2572e4
    st gp[656] = add1
    1
    loop
              mov 656(esi),ebx                ecx(state) ebx(add1) esi(gp)
        0x258ff3:
              jmp 0x0                        
    0x258ff8  [epilogue]                     
              mov esp,ebp                    
              pop ebp                        
              pop ebp                        
              pop ebx                        
              pop esi                        
              pop edi                        
              ret                            
fragment 0x257004:
ENTRY: G1 
recording completed at 2a.js:2@24 via closeLoop
Looking for compat peer 2@24, from 0x257004 (ip: 0x30dbf8, hits=2)
checking vm types 0x257004 (ip: 0x30dbf8): global0=I/I 
entering trace at 2a.js:2@24, native stack slots: 3 code: 0x258f60
global: int<2> 
stack: 
leaving trace at 2a.js:2@32, op=eq, lr=0x2571e8, exitType=0, sp=2, calldepth=0, cycles=51557
int<2> 
stack0=int<2> stack1=int<2> 
trying to attach another branch to the tree (hits = 0)
recording starting from 2a.js:2@32
globalObj=0x294000, shape=147
import vp=0x811738 name=$global0 type=int flags=0
import vp=0x814624 name=$stack0 type=int flags=0
import vp=0x814628 name=$stack1 type=int flags=0
00032:   2  eq
    start
    state = param 0 ecx
    0
    sp = ld state[0]
    4
    rp = ld state[4]
    12
    cx = ld state[12]
    8
    gp = ld state[8]
    16
    eos = ld state[16]
    20
    eor = ld state[20]
    40
    globalObj = ld state[40]
    656
    ld3 = ld gp[656]
    $global0 = i2f ld3
    ld4 = ld sp[0]
    $stack0 = i2f ld4
    ld5 = ld sp[8]
    $stack1 = i2f ld5
    eq3 = eq ld4, ld5
    xf2: xf eq3 -> 0:32 sp+16 rp+0

00036:   2  enterblock depth 0 {y: 0}
00039:   2  newarray 0
    sti sp[0] = eq3
    JSVAL_TO_BOOLEAN(JSVAL_VOID)
    sti sp[0] = JSVAL_TO_BOOLEAN(JSVAL_VOID)
    proto
    js_NewUninitializedArray1 = js_NewUninitializedArray ( cx proto 0 )

    eq4 = eq js_NewUninitializedArray1, 0
    xt5: xt eq4 -> 0:39 sp+8 rp+0

    116
    ld6 = ld cx[116]
    #0x111f4
    ld7 = ld ld6[#0x111f4]
    eq5 = eq ld7, 0
    xf3: xf eq5 -> 0:39 sp+8 rp+0

00043:   2  iter 1
00000:  callprop "__iterator__"
    sti sp[8] = js_NewUninitializedArray1
    ld8 = ld js_NewUninitializedArray1[8]
    ld9 = ld ld8[0]
    ops = ld ld9[4]
    ld10 = ld ops[16]
    OP(&js_ObjectOps)
    guard(native-map) = eq ld10, OP(&js_ObjectOps)
    xf4: xf guard(native-map) -> 1:43 sp+16 rp+0

    shape = ld ld9[16]
    124
    guard(kshape) = eq shape, 124
    xf5: xf guard(kshape) -> 1:43 sp+16 rp+0

    ld11 = ld ld8[8]
    eq6 = eq ld11, 0
    xt6: xt eq6 -> 1:43 sp+16 rp+0

    ld12 = ld ld11[0]
    ops = ld ld12[4]
    ld13 = ld ops[0]
    OP(&js_ObjectOps)
    guard(native-map) = eq ld13, OP(&js_ObjectOps)
    xf6: xf guard(native-map) -> 1:43 sp+16 rp+0

    shape = ld ld12[16]
    150
    guard(vshape) = eq shape, 150
    xf7: xf guard(vshape) -> 1:43 sp+16 rp+0

00000:  int8 1
00000:  call 1
abort: 6665: unknown native
Abort recording (line 2, pc 43): call.
Assertion failure: !JSVAL_IS_PRIMITIVE(regs.sp[-2]), at ../jsinterp.cpp:3237
Trace/BPT trap
Group: core-security
Whiteboard: [sg:critical?]
(Assignee)

Comment 6

9 years ago
Created attachment 361003 [details] [diff] [review]
Fix
Attachment #361003 - Flags: review?(brendan)
Comment on attachment 361003 [details] [diff] [review]
Fix

>     .imacro for_in_native                           # obj
>         callbuiltin (JSBUILTIN_ObjectToIterator)    # fun obj
>         int8 (JSITER_ENUMERATE)                     # fun obj flags
>         call 1                                      # iterobj
>+        iteratortop                                 # iterobj
>         push                                        # iterobj undef
>         stop
>     .end
> 
>     .imacro for_each_native                         # obj
>         callbuiltin (JSBUILTIN_ObjectToIterator)    # fun obj
>         int8 (JSITER_ENUMERATE|JSITER_FOREACH)      # fun obj flags
>         call 1                                      # iterobj
>+        iteratortop                                 # iterobj
>         push                                        # iterobj undef
>         stop
>     .end

These two iteratortops are unnecessary as you noted.

>           END_CASE(JSOP_PRIMTOP)
> 
>+          BEGIN_CASE(JSOP_ITERATORTOP)
>+            lval = FETCH_OPND(-1);
>+            if (JSVAL_IS_PRIMITIVE(lval)) {
>+                atom = cx->runtime->atomState.iteratorAtom;
>+                const char *printable = js_AtomToPrintableString(cx, atom);
>+                if (printable) {
>+                    js_ReportValueError2(cx, JSMSG_BAD_ITERATOR_RETURN,
>+                                         JSDVG_SEARCH_STACK, lval, NULL,

s/JSDVG_SEARCH_STACK/-1/

Here in the shadow of PRIMTOP I would rather we add JSOP_OBJTOP and give it an immediate, which can be JSMSG_BAD_ITERATOR_RETURN in parens in the imacro source language. JOF_UINT8 is enough given current js.msg:

$ grep MSG_DEF js.msg|wc -l
     231

but to have room for growth, why not use JOF_UINT16.

/be
(Assignee)

Comment 8

9 years ago
Created attachment 361022 [details] [diff] [review]
Updated to comments
Attachment #361003 - Attachment is obsolete: true
Attachment #361022 - Flags: review?(brendan)
Attachment #361003 - Flags: review?(brendan)
(Assignee)

Comment 9

9 years ago
Created attachment 361024 [details] [diff] [review]
Updated even more

Now OBJTOP doesn't depend on being used to report iterator messages.
Attachment #361022 - Attachment is obsolete: true
Attachment #361024 - Flags: review?(brendan)
Attachment #361022 - Flags: review?(brendan)
(Assignee)

Comment 10

9 years ago
I also made the comment above JSOP_OBJTOP read:
/*
 * Ensure that the value on the top of the stack is an object. The one
 * argument is an error message, defined in js.msg, that takes one parameter
 * (the decompilation of the primitive value).
 */
Attachment #361024 - Flags: review?(brendan) → review+
Comment on attachment 361024 [details] [diff] [review]
Updated even more

>+          BEGIN_CASE(JSOP_OBJTOP)
>+            lval = FETCH_OPND(-1);
>+            if (JSVAL_IS_PRIMITIVE(lval)) {
>+                js_ReportValueError(cx, GET_UINT16(regs.pc), -1,
>+                                    lval, NULL);

That so fits on one line in the new world order, maybe even in the old -- let's check:

..12345678901234567890123456789012345678901234567890123456789012345678901234567890
>+                js_ReportValueError(cx, GET_UINT16(regs.pc), -1, lval, NULL);

Yup, even in the old.

r=me with that nit picked.

/be
(Assignee)

Comment 12

9 years ago
http://hg.mozilla.org/tracemonkey/rev/7da027316584
Whiteboard: [sg:critical?] → [sg:critical?] fixed-in-tracemonkey

Comment 13

9 years ago
http://hg.mozilla.org/mozilla-central/rev/7da027316584
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Comment 14

9 years ago
Created attachment 362938 [details]
js1_7/extensions/regress-469405-01.js

Comment 15

9 years ago
Created attachment 362939 [details]
js1_7/extensions/regress-469405-02.js

Comment 16

9 years ago
I couldn't reproduce the original problem on mac/shell with either testcase.
Flags: in-testsuite+
Flags: in-litmus-

Comment 17

9 years ago
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ab0950f97a9c
Keywords: fixed1.9.1

Comment 18

9 years ago
note, i was able to reproduce the crash in opt and assertion in debug for browser on 1.9.1 for test 01 as recently as 2/27, but this is now fixed.

v 1.9.1, 1.9.2
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
Group: core-security
Flags: wanted1.9.0.x-

Comment 19

8 years ago
test checked into 1.9.0, 1.9.1, 1.9.2, tracemonkey. 1.9.3 will get picked up in the next merge.
You need to log in before you can comment on or make changes to this bug.