1267439 - crash in js::frontend::BytecodeEmitter::emitTree

Status: RESOLVED INCOMPLETE

(Core :: JavaScript Engine, defect)

Description

[Liz Henry (:lizzard) (relman/hg->git project)]

This bug was filed from the Socorro interface and is 
report bp-0d00c81b-2072-4af9-a083-fb9e72160424.
=============================================================
Crash reported on SUMO (https://support.mozilla.org/en-US/questions/1119873#answer-869623). 
The signature is showing up in low volume in current versions but is often marked as high exploitability. 
See also https://bugzilla.mozilla.org/show_bug.cgi?id=1186962

Comment 1

[Kevin Brosnan [Ex-Mozilla]]

Guigs is rmcguigan@mozilla she may be the best go between to this user that experiences the crash repeatedly.

Comment 2

[Liz Henry (:lizzard) (relman/hg->git project)]

waldo, might you take a look here since you are familiar with the bugs with similar signatures?

Comment 3

[Tooru Fujisawa [:arai]]

Reading from the crash dump, looks like, the crash happens in the |pn->getKind()| there, where pn is 0x000003f6.

https://hg.mozilla.org/releases/mozilla-release/file/e35da3da61cb/js/src/frontend/BytecodeEmitter.cpp#l8249
> bool
> BytecodeEmitter::emitTree(ParseNode* pn, EmitLineNumberNote emitLineNote)
> {
> ...
>     if (emitLineNote == EMIT_LINENOTE && pn->getKind() != PNK_WHILE && pn->getKind() != PNK_FOR &&
>         !updateLineNumberNotes(pn->pn_pos.begin))
>         return false;

Here's related assembly from Firefox 45.0.2 32bit on windows (comments are just a guess)

  012b6f1f 33db            xor     ebx,ebx               ; 0h = EMIT_LINENOTE
  012b6f21 8b7d08          mov     edi,dword ptr [ebp+8]
  012b6f24 395d0c          cmp     dword ptr [ebp+0Ch],ebx
  012b6f27 751f            jne     012b6f48
->012b6f29 8b07            mov     eax,dword ptr [edi]   ; pn->pn_type
  012b6f2b 25ffff0000      and     eax,0FFFFh
  012b6f30 83f829          cmp     eax,29h               ; 29h == PNK_FOR
  012b6f33 7413            je      012b6f48
  012b6f35 83f827          cmp     eax,27h               ; 27h == PNK_WHILE
  012b6f38 740e            je      012b6f48
  012b6f3a ff7704          push    dword ptr [edi+4]     ; pn->pn_pos.begin
  012b6f3d 8bce            mov     ecx,esi
  012b6f3f e8ac6c0000      call    012bdbf0
  012b6f44 84c0            test    al,al
  012b6f46 7438            je      012b6f80
  012b6f48 8b07            mov     eax,dword ptr [edi]

But I'm not sure how this could happen, with this information.
(maybe a remainder of the Parser::clone* issue?)

Comment 4

[Daniel Veditz [:dveditz]]

Some of the crashes with this signature are clearly use-after-frees, some are null or near-null derefs, and some appear to be random addresses. There's -something- wrong here, but without a testcase it's going to be hard to figure out.

Comment 5

[Andrew McCreight [:mccr8]]

I don't see any crashes on 49 or 50, so maybe something got fixed?

Comment 6

[Liz Henry (:lizzard) (relman/hg->git project)]

Arai, can you have a look at the current crashes on 49 and up to see if there's still a security issue? Thanks.

Comment 7

[Tooru Fujisawa [:arai]]

I checked recent crashes on 49, and in almost all cases, stack trace looks broken.
the bottom of the stack trace is not main nor helper thread. and also it doesn't look like JIT call...
I wonder how it's executed.  or perhaps stack got broken while executing it?

anyway, will check assembly shortly.

Comment 8

[Tooru Fujisawa [:arai]]

Here's disassembled code from Firefox 49.0.1
function_offset 0x30 is 0FAD458C.

  0FAD455C  push        ebp
  0FAD455D  mov         ebp,esp
  0FAD455F  and         esp,0FFFFFFF8h
  0FAD4562  push        ecx
  0FAD4563  push        ebx
  0FAD4564  push        esi
  0FAD4565  mov         esi,ecx
  0FAD4567  push        edi
  0FAD4568  mov         ecx,dword ptr [esi+4]
  0FAD456B  call        js::GetNativeStackLimit (0FAD4A30h)
  0FAD4570  lea         edx,[esp+0Ch]
  0FAD4574  cmp         edx,eax
  0FAD4576  jbe         `js::irregexp::RegExpEmpty::GetInstance'::`2'::`dynamic atexit destructor for 'instance''+6C619h (100722C9h)
  0FAD457C  inc         dword ptr [esi+124h]
  0FAD4582  xor         ebx,ebx
  0FAD4584  mov         edi,dword ptr [pn]
  0FAD4587  cmp         dword ptr [emitLineNote],ebx
  0FAD458A  jne         js::frontend::BytecodeEmitter::emitTree+4Fh (0FAD45ABh)
> 0FAD458C  mov         eax,dword ptr [edi]
  0FAD458E  and         eax,0FFFFh
  0FAD4593  cmp         eax,29h
  0FAD4596  je          js::frontend::BytecodeEmitter::emitTree+4Fh (0FAD45ABh)
  0FAD4598  cmp         eax,27h
  0FAD459B  je          js::frontend::BytecodeEmitter::emitTree+4Fh (0FAD45ABh)
  0FAD459D  push        dword ptr [edi+4]
  0FAD45A0  mov         ecx,esi
  0FAD45A2  call        js::frontend::BytecodeEmitter::updateLineNumberNotes (0FAD4A80h)
  0FAD45A7  test        al,al
  0FAD45A9  je          js::frontend::BytecodeEmitter::emitTree+87h (0FAD45E3h)
  0FAD45AB  mov         eax,dword ptr [edi]
  0FAD45AD  and         eax,0FFFFh
  0FAD45B2  cmp         eax,86h
  0FAD45B7  ja          js::frontend::BytecodeEmitter::emitTree+78h (0FAD45D4h)
  0FAD45B9  movzx       eax,byte ptr [eax+0FAD49A4h]
  0FAD45C0  jmp         dword ptr [eax*4+0FAD48B0h]
  0FAD45C7  push        ebx
  0FAD45C8  push        edi
  0FAD45C9  mov         ecx,esi
  0FAD45CB  call        js::frontend::BytecodeEmitter::emitNameOp (0FAC85D0h)
  0FAD45D0  test        al,al
  0FAD45D2  je          js::frontend::BytecodeEmitter::emitTree+87h (0FAD45E3h)

So, it's still the same issue.  pn holds random address and the code is trying to dereference it.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

No reports of this after version 50. Wondering if the signature changed or if we legitimately fixed it somewhere in the 51 cycle.

Comment 10

[Andrew McCreight [:mccr8]]

I see a few crashes in 50 with poison values:
  bp-3e875580-975d-431d-a6da-a66452170106
  bp-f5e3ab16-b4a1-439a-ad6e-0e7dd2170105

The big front end rewrite (bug 1263355) landed in 51 so maybe that helped.