Closed Bug 1269729 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: JavaScript Engine, defect)

ARM
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla49
Tracking Status
firefox46 --- wontfix
firefox47 --- verified
firefox48 --- verified
firefox49 --- verified
firefox-esr45 47+ fixed

People

(Reporter: decoder, Assigned: efaust)

Details

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

Attachments

(1 file)

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.
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
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.
CC jolesen for review.
Attached patch FixSplinter Review
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+
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+
Yes, 45 is affected. I will nominate for all once this settles on trunk.
https://hg.mozilla.org/mozilla-central/rev/374889c1ff7d
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Group: javascript-core-security → core-security-release
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 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+
JSBugMon: This bug has been automatically verified fixed on Fx48
Whiteboard: [jsbugmon:update] → [jsbugmon:update][adv-main47+][adv-esr45.2+]
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.