The default bug view has changed. See this FAQ.

Assertion failure: aIndex < mLength, at dist/include/mozilla/Vector.h:439 with OOM

VERIFIED FIXED in Firefox 47

Status

()

Core
JavaScript Engine
--
critical
VERIFIED FIXED
11 months ago
6 months ago

People

(Reporter: decoder, Assigned: efaust)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla49
ARM
Linux
assertion, regression, sec-high, testcase
Points:
---

Firefox Tracking Flags

(firefox46 wontfix, firefox47 verified, firefox48 verified, firefox49 verified, firefox-esr4547+ fixed)

Details

(Whiteboard: [jsbugmon:update][adv-main47+][adv-esr45.2+])

Attachments

(1 attachment)

(Reporter)

Description

11 months ago
The following testcase crashes on mozilla-central revision 77cead2cd203 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --target=i686-pc-linux-gnu --disable-tests --enable-simulator=arm --enable-debug, run with --fuzzing-safe --thread-count=2 --ion-eager --ion-offthread-compile=off):

loadFile("");
loadFile("");
loadFile(`
  (function() {
    'use asm'
    function _main() {
      switch (0) {
       case 1:
       case 20:
      }
    }
  })
`);
function loadFile(lfVarx) {
    try {
      oomTest(function() {
          evaluate(lfVarx);
      })
    } catch (lfVare) {}
} 



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x084b74ca in operator[] (aIndex=19, this=0xf54ec4f8) at js/src/debugarmsim/dist/include/mozilla/Vector.h:439
#0  0x084b74ca in operator[] (aIndex=19, this=0xf54ec4f8) at js/src/debugarmsim/dist/include/mozilla/Vector.h:439
#1  codeLabel (i=19, this=0xf54ec4e0) at js/src/jit/arm/CodeGenerator-arm.cpp:1063
#2  js::jit::CodeGeneratorARM::visitOutOfLineTableSwitch (this=0xffff6c30, ool=0xf54ec4e0) at js/src/jit/arm/CodeGenerator-arm.cpp:1080
#3  0x084c4a78 in js::jit::OutOfLineCodeBase<js::jit::CodeGeneratorARM>::generate (this=0xf54ec4e0, codegen=0xffff6c30) at js/src/jit/shared/CodeGenerator-shared.h:582
#4  0x08512e28 in js::jit::CodeGeneratorShared::generateOutOfLineCode (this=this@entry=0xffff6c30) at js/src/jit/shared/CodeGenerator-shared.cpp:182
#5  0x084bf31e in js::jit::CodeGeneratorARM::generateOutOfLineCode (this=this@entry=0xffff6c30) at js/src/jit/arm/CodeGenerator-arm.cpp:117
#6  0x082cbe86 in js::jit::CodeGenerator::generateAsmJS (this=this@entry=0xffff6c30, offsets=0xf7a4d7c8) at js/src/jit/CodeGenerator.cpp:8784
#7  0x08241ae3 in js::wasm::IonCompileFunction (task=0xf7a4d000) at js/src/asmjs/WasmIonCompile.cpp:3242
#8  0x081cd38f in js::wasm::ModuleGenerator::finishFuncDef (this=0xffff7ffc, funcIndex=0, generateTime=0, fg=fg@entry=0xffff7908) at js/src/asmjs/WasmGenerator.cpp:816
#9  0x081dcab3 in finish (generateTime=<optimized out>, funcIndex=<optimized out>, this=0xffff7900) at js/src/asmjs/AsmJS.cpp:2663
#10 CheckFunction (m=...) at js/src/asmjs/AsmJS.cpp:6696
#11 0x081def33 in CheckFunctions (m=...) at js/src/asmjs/AsmJS.cpp:6727
#12 CheckModule (cx=cx@entry=0xf7a74020, parser=..., stmtList=stmtList@entry=0xf7a813f0, moduleObj=moduleObj@entry=..., time=time@entry=0xffff8a40, slowFuncs=slowFuncs@entry=0xffff8aa0) at js/src/asmjs/AsmJS.cpp:6938
#13 0x081e39aa in js::CompileAsmJS (cx=0xf7a74020, parser=..., stmtList=stmtList@entry=0xf7a813f0, validated=validated@entry=0xffff8be0) at js/src/asmjs/AsmJS.cpp:8219
#14 0x08136bd5 in js::frontend::Parser<js::frontend::FullParseHandler>::asmJS (this=0xffffa3c0, list=0xf7a813f0) at js/src/frontend/Parser.cpp:3441
[...]
#41 0x0813f4ce in js::frontend::Parser<js::frontend::FullParseHandler>::globalBody (this=this@entry=0xffffa3c0) at js/src/frontend/Parser.cpp:1107
#42 0x0887f03b in BytecodeCompiler::compileScript (this=this@entry=0xffff9ef4, scopeChain=scopeChain@entry=..., evalCaller=evalCaller@entry=...) at js/src/frontend/BytecodeCompiler.cpp:531
#43 0x0887f75a in js::frontend::CompileScript (cx=cx@entry=0xf7a74020, alloc=0xf7a3c1a4, scopeChain=scopeChain@entry=..., enclosingStaticScope=enclosingStaticScope@entry=..., evalCaller=evalCaller@entry=..., options=..., srcBuf=..., source_=source_@entry=0x0, extraSct=extraSct@entry=0x0, sourceObjectOut=sourceObjectOut@entry=0x0) at js/src/frontend/BytecodeCompiler.cpp:742
#44 0x08541ce1 in Compile (cx=cx@entry=0xf7a74020, options=..., scopeOption=scopeOption@entry=HasSyntacticScope, srcBuf=..., script=script@entry=...) at js/src/jsapi.cpp:4006
#45 0x085422f6 in Compile (script=..., length=123, chars=0xf55fe300 u"\n  (function() {\n    'use asm'\n    function _main() {\n      switch (0) {\n       case 1:\n       case 20:\n      }\n    }\n  })\n", scopeOption=HasSyntacticScope, options=..., cx=0xf7a74020) at js/src/jsapi.cpp:4015
#46 JS::Compile (cx=0xf7a74020, options=..., chars=0xf55fe300 u"\n  (function() {\n    'use asm'\n    function _main() {\n      switch (0) {\n       case 1:\n       case 20:\n      }\n    }\n  })\n", length=123, script=script@entry=...) at js/src/jsapi.cpp:4074
#47 0x080f21b7 in Evaluate (cx=cx@entry=0xf7a74020, argc=argc@entry=1, vp=vp@entry=0xf59ffcb8) at js/src/shell/js.cpp:1481
[...]
#60 0x0854b440 in JS_CallFunction (cx=0xf7a74020, obj=..., fun=fun@entry=..., args=..., rval=rval@entry=...) at js/src/jsapi.cpp:2883
#61 0x088487bc in OOMTest (cx=cx@entry=0xf7a74020, argc=argc@entry=1, vp=vp@entry=0xf59ffdd8) at js/src/builtin/TestingFunctions.cpp:1310
#62 0x084fafb3 in js::jit::Simulator::softwareInterrupt (this=0xf7a1c000, instr=0xf553b5d4) at js/src/jit/arm/Simulator-arm.cpp:2359
[...]
#92 main (argc=6, argv=0xffffcc84, envp=0xffffcca0) at js/src/shell/js.cpp:7483
eax	0x0	0
ebx	0x988cffc	159961084
ecx	0xf7e3a88c	-136075124
edx	0x0	0
esi	0xf54ec108	-179388152
edi	0xf54eb1e0	-179392032
ebp	0xffff6878	4294928504
esp	0xffff6810	4294928400
eip	0x84b74ca <js::jit::CodeGeneratorARM::visitOutOfLineTableSwitch(js::jit::OutOfLineTableSwitch*)+970>
=> 0x84b74ca <js::jit::CodeGeneratorARM::visitOutOfLineTableSwitch(js::jit::OutOfLineTableSwitch*)+970>:	movl   $0x1b7,0x0
   0x84b74d4 <js::jit::CodeGeneratorARM::visitOutOfLineTableSwitch(js::jit::OutOfLineTableSwitch*)+980>:	call   0x8108050 <abort()>


The assertion indicates something being out of bounds which is likely a security problem. Marking s-s.

Updated

11 months ago
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]

Comment 1

11 months ago
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/9c365490d4ce
user:        Jon Coppeard
date:        Tue Oct 13 13:37:07 2015 +0100
summary:     Bug 1212469 - Make oomTest() into a shell function r=nbp

This iteration took 337.169 seconds to run.

Updated

11 months ago
status-firefox46: --- → wontfix
status-firefox47: --- → affected
status-firefox48: --- → affected
status-firefox-esr45: --- → affected

Updated

11 months ago
Keywords: sec-high
(Assignee)

Comment 2

11 months ago
CC jolesen for review.
(Assignee)

Comment 3

11 months ago
Created attachment 8748323 [details] [diff] [review]
Fix

I know we talked about this on IRC, but since it's marked ss, we should have a paper trail.
Assignee: nobody → efaustbmo
Status: NEW → ASSIGNED
Attachment #8748323 - Flags: review?(jolesen)
Comment on attachment 8748323 [details] [diff] [review]
Fix

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

LGTM
Attachment #8748323 - Flags: review?(jolesen) → review+
(Assignee)

Comment 5

11 months ago
Comment on attachment 8748323 [details] [diff] [review]
Fix

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

This prevents an out of bounds read on a heap vector with OOM. Since the values read out of bounds are used as offsets for code labels, one could in theory jump to random code.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

Definitely not.

Which older supported branches are affected by this flaw?

47+ marked affected at least.

If not all supported branches, which bug introduced the flaw?

It requires OOM, so it's likely been there forever.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

This should just drop in.

How likely is this patch to cause regressions; how much testing does it need?

We can let it bake on Inbound, but this is very safe. It just adds robustness to an error case.
Attachment #8748323 - Flags: sec-approval?
Comment on attachment 8748323 [details] [diff] [review]
Fix

sec-approval+ for trunk.

You say 47+ is affected in the sec-approval request but ESR45 is flagged as affected. Is ESR45 affected?

We should get aurora and beta branches nominated to go in once this has cleared trunk. If ESR45 is affected, we're going to want it there as well.
Attachment #8748323 - Flags: sec-approval? → sec-approval+
(Assignee)

Comment 7

11 months ago
Yes, 45 is affected. I will nominate for all once this settles on trunk.
(Assignee)

Comment 8

11 months ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/374889c1ff7d
https://hg.mozilla.org/mozilla-central/rev/374889c1ff7d
Status: ASSIGNED → RESOLVED
Last Resolved: 11 months ago
status-firefox49: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49

Updated

11 months ago
Status: RESOLVED → VERIFIED
status-firefox49: fixed → verified

Comment 10

11 months ago
JSBugMon: This bug has been automatically verified fixed.

Updated

11 months ago
Group: javascript-core-security → core-security-release
(Assignee)

Comment 11

11 months ago
Comment on attachment 8748323 [details] [diff] [review]
Fix

Approval Request Comment
[Feature/regressing bug #]: Been around for a long time. Unchecked oom condition
[User impact if declined]: Poor OOM handling. Out of bounds access used as code jump target
[Describe test coverage new/current, TreeHerder]: Baked on inbound since landing
[Risks and why]: Very low. Correctly handle error case.
[String/UUID change made/needed]: N/A
Attachment #8748323 - Flags: approval-mozilla-esr45?
Attachment #8748323 - Flags: approval-mozilla-beta?
Attachment #8748323 - Flags: approval-mozilla-aurora?

Comment 12

11 months ago
Comment on attachment 8748323 [details] [diff] [review]
Fix

Sec-high issue, Aurora48+, Beta47+, ESR45+
Attachment #8748323 - Flags: approval-mozilla-esr45?
Attachment #8748323 - Flags: approval-mozilla-esr45+
Attachment #8748323 - Flags: approval-mozilla-beta?
Attachment #8748323 - Flags: approval-mozilla-beta+
Attachment #8748323 - Flags: approval-mozilla-aurora?
Attachment #8748323 - Flags: approval-mozilla-aurora+

Updated

11 months ago
tracking-firefox-esr45: --- → 47+

Comment 13

11 months ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/75eba1454695
https://hg.mozilla.org/releases/mozilla-beta/rev/4f79d42e0293
https://hg.mozilla.org/releases/mozilla-esr45/rev/9ec3d076fbee
status-firefox47: affected → fixed
status-firefox48: affected → fixed
status-firefox-esr45: affected → fixed

Updated

10 months ago
status-firefox48: fixed → verified

Comment 14

10 months ago
JSBugMon: This bug has been automatically verified fixed on Fx48

Updated

10 months ago
Whiteboard: [jsbugmon:update] → [jsbugmon:update][adv-main47+][adv-esr45.2+]

Updated

10 months ago
status-firefox47: fixed → verified

Comment 15

10 months ago
JSBugMon: This bug has been automatically verified fixed on Fx47
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.