Closed
Bug 1369994
Opened 7 years ago
Closed 7 years ago
Assertion failure: isInt32(), at dist/include/js/Value.h:605
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
VERIFIED
FIXED
mozilla56
People
(Reporter: decoder, Unassigned)
References
(Blocks 1 open bug)
Details
(Keywords: assertion, sec-high, testcase, Whiteboard: [jsbugmon:][adv-main55+][adv-esr52.3+])
Crash Data
Attachments
(1 file, 2 obsolete files)
2.29 KB,
patch
|
jonco
:
review+
jcristau
:
approval-mozilla-beta+
abillings
:
approval-mozilla-esr52+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision aeb3d0ca558f (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --thread-count=2 --ion-offthread-compile=off --baseline-eager): var lfLogBuffer = ` function f() { var i32 = new Int32Array(1); var f32 = new Float32Array(i32.buffer); for (var i = 0; i < 3; i++) { var { regExp, get, } = gczeal(9,10) ? (yield) : (yield) = call(f32, "i32.store", []); } } f(); `; loadFile(lfLogBuffer); function loadFile(lfVarx) { try { oomTest(function() { eval(lfVarx); }); } catch (lfVare) {} } Backtrace: Program terminated with signal SIGSEGV, Segmentation fault. #0 0x000000000067ffc0 in JS::Value::toInt32 (this=<optimized out>) at dist/include/js/Value.h:605 #1 js::TypedArrayObject::length (this=this@entry=0x7ff2e4299100) at js/src/vm/TypedArrayObject.h:164 #2 0x0000000000c43f11 in js::TypedArrayObject::assertZeroLengthArrayData (this=0x7ff2e4299100) at js/src/vm/TypedArrayObject.cpp:147 #3 0x0000000000c43f7f in js::TypedArrayObject::elements (this=0x7ff2e4299100) at js/src/vm/TypedArrayObject.h:173 #4 js::TypedArrayObject::hasInlineElements (this=0x7ff2e4299100) at js/src/vm/TypedArrayObject.cpp:272 #5 0x0000000000c486b0 in js::TypedArrayObject::objectMoved (obj=<optimized out>, old=0x7ff2e4299100) at js/src/vm/TypedArrayObject.cpp:194 #6 0x00000000009c81c3 in RelocateCell (thingSize=96, thingKind=js::gc::AllocKind::OBJECT8_BACKGROUND, src=0x7ff2e4299100, zone=0x7ff2e40e4000) at js/src/jsgc.cpp:1993 #7 RelocateArena (sliceBudget=..., arena=0x7ff2e4299000) at js/src/jsgc.cpp:2023 #8 js::gc::ArenaList::relocateArenas (this=this@entry=0x7ff2e40e4238, toRelocate=0x0, toRelocate@entry=0x7ff2e42cb000, relocated=0x7ff2e42cb000, sliceBudget=..., stats=...) at js/src/jsgc.cpp:2063 #9 0x00000000009c93ae in js::gc::ArenaLists::relocateArenas (this=this@entry=0x7ff2e40e40b0, zone=zone@entry=0x7ff2e40e4000, relocatedListOut=@0x7ffe1a1569d8: 0x7ff2e4239000, reason=reason@entry=JS::gcreason::DEBUG_GC, sliceBudget=..., stats=...) at js/src/jsgc.cpp:2110 #10 0x00000000009c954b in js::gc::GCRuntime::relocateArenas (this=this@entry=0x7ff2e595e5f8, zone=zone@entry=0x7ff2e40e4000, reason=reason@entry=JS::gcreason::DEBUG_GC, relocatedListOut=@0x7ffe1a1569d8: 0x7ff2e4239000, sliceBudget=...) at js/src/jsgc.cpp:2147 #11 0x00000000009c98e7 in js::gc::GCRuntime::compactPhase (this=this@entry=0x7ff2e595e5f8, reason=reason@entry=JS::gcreason::DEBUG_GC, sliceBudget=..., lock=...) at js/src/jsgc.cpp:5727 #12 0x00000000009d7939 in js::gc::GCRuntime::incrementalCollectSlice (this=this@entry=0x7ff2e595e5f8, budget=..., reason=reason@entry=JS::gcreason::DEBUG_GC, lock=...) at js/src/jsgc.cpp:6228 #13 0x00000000009d8b94 in js::gc::GCRuntime::gcCycle (this=this@entry=0x7ff2e595e5f8, nonincrementalByAPI=nonincrementalByAPI@entry=false, budget=..., reason=reason@entry=JS::gcreason::DEBUG_GC) at js/src/jsgc.cpp:6465 #14 0x00000000009d9488 in js::gc::GCRuntime::collect (this=this@entry=0x7ff2e595e5f8, nonincrementalByAPI=nonincrementalByAPI@entry=false, budget=..., reason=reason@entry=JS::gcreason::DEBUG_GC) at js/src/jsgc.cpp:6614 #15 0x00000000009dac1c in js::gc::GCRuntime::runDebugGC (this=this@entry=0x7ff2e595e5f8) at js/src/jsgc.cpp:7151 #16 0x0000000000d28ac5 in js::gc::GCRuntime::gcIfNeededAtAllocation (this=0x7ff2e595e5f8, cx=cx@entry=0x7ff2e5924000) at js/src/gc/Allocator.cpp:230 #17 0x0000000000d36918 in js::gc::GCRuntime::checkAllocatorState<(js::AllowGC)1> (this=<optimized out>, cx=0x7ff2e5924000, kind=js::gc::AllocKind::OBJECT_LIMIT) at js/src/gc/Allocator.cpp:191 #18 0x0000000000d3a092 in js::Allocate<JSScript, (js::AllowGC)1> (cx=cx@entry=0x7ff2e5924000) at js/src/gc/Allocator.cpp:142 #19 0x0000000000a2c2d4 in JSScript::Create (cx=0x7ff2e5924000, options=..., sourceObject=..., sourceObject@entry=..., bufStart=bufStart@entry=0, bufEnd=246, toStringStart=0, toStringEnd=246) at js/src/jsscript.cpp:2627 #20 0x0000000000a8163e in BytecodeCompiler::createScript (this=0x7ffe1a157340, toStringStart=<optimized out>, toStringEnd=<optimized out>) at js/src/frontend/BytecodeCompiler.cpp:272 #21 0x0000000000aadc7d in BytecodeCompiler::createScript (this=0x7ffe1a157340) at js/src/frontend/BytecodeCompiler.cpp:266 #22 BytecodeCompiler::compileScript (this=this@entry=0x7ffe1a157340, environment=..., environment@entry=..., sc=sc@entry=0x7ffe1a1572e0) at js/src/frontend/BytecodeCompiler.cpp:336 #23 0x0000000000aae46a in BytecodeCompiler::compileEvalScript (enclosingScope=..., environment=..., this=0x7ffe1a157340) at js/src/frontend/BytecodeCompiler.cpp:402 #24 js::frontend::CompileEvalScript (cx=cx@entry=0x7ff2e5924000, alloc=..., environment=environment@entry=..., enclosingScope=..., options=..., srcBuf=..., sourceObjectOut=0x0) at js/src/frontend/BytecodeCompiler.cpp:601 #25 0x000000000056bf90 in EvalKernel (cx=cx@entry=0x7ff2e5924000, v=..., v@entry=..., evalType=evalType@entry=DIRECT_EVAL, caller=..., env=env@entry=..., pc=<optimized out>, vp=...) at js/src/builtin/Eval.cpp:318 #26 0x000000000056c75e in js::DirectEval (cx=cx@entry=0x7ff2e5924000, v=..., vp=vp@entry=...) at js/src/builtin/Eval.cpp:438 #27 0x000000000060c7f0 in js::jit::DoCallFallback (cx=0x7ff2e5924000, frame=0x7ffe1a158b38, stub_=<optimized out>, argc=<optimized out>, vp=0x7ffe1a158ae8, res=...) at js/src/jit/BaselineIC.cpp:2437 #28 0x00000e1b84031d77 in ?? () rax 0x0 0 rbx 0x7ff2e4299100 140681186742528 rcx 0x7ff2e5c5aa2d 140681213749805 rdx 0x0 0 rsi 0x7ff2e5f29770 140681216694128 rdi 0x7ff2e5f28540 140681216689472 rbp 0x7ffe1a1565d0 140729336030672 rsp 0x7ffe1a1565d0 140729336030672 r8 0x7ff2e5f29770 140681216694128 r9 0x7ff2e701a740 140681234458432 r10 0x58 88 r11 0x7ff2e5bd1750 140681213187920 r12 0x7ff2e42ef280 140681187095168 r13 0x7ff2e4299100 140681186742528 r14 0x60 96 r15 0x7ff2e42ef280 140681187095168 rip 0x67ffc0 <js::TypedArrayObject::length() const+112> => 0x67ffc0 <js::TypedArrayObject::length() const+112>: movl $0x0,0x0 0x67ffcb <js::TypedArrayObject::length() const+123>: ud2 Marking s-s because the test uses gczeal and TypedArrays plus the assertion shows a type problem, so this is very likely a security issue. The testcase is intermittent but should reproduce in less than 10 runs. If you could figure out how to make this testcase deterministic that would be good for future testing.
Updated•7 years ago
|
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:bisect]
Comment 1•7 years ago
|
||
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
Updated•7 years ago
|
Whiteboard: [jsbugmon:bisect] → [jsbugmon:]
Comment hidden (obsolete) |
Comment 2 is probably invalid due to the intermittent testcase. Since this involves TypedArrays, setting needinfo? from Steve as a start.
Flags: needinfo?(sphink)
Comment 4•7 years ago
|
||
It looks like compacting GC is involved, so maybe Jon could take a look.
Flags: needinfo?(jcoppeard)
Keywords: sec-high
Updated•7 years ago
|
Flags: needinfo?(jcoppeard)
Comment 5•7 years ago
|
||
The condition in the assertion doesn't always hold because we can fail initialisation of template objects part way through. The patch shuffles around the initialisation code so that doesn't happen.
Comment 6•7 years ago
|
||
Comment on attachment 8878468 [details] [diff] [review] bug1369994-typed-array-init Review of attachment 8878468 [details] [diff] [review]: ----------------------------------------------------------------- Do you think this is s-s? If yes we shouldn't add the test.
Attachment #8878468 -
Flags: review?(jdemooij) → review+
Comment 7•7 years ago
|
||
I can't immediately see how this could create a vulnerability, but leaving s-s since typed arrays are sufficiently complicated that there could be one. I'll remove the test.
Group: javascript-core-security
Updated•7 years ago
|
Group: javascript-core-security
Comment 8•7 years ago
|
||
[Security approval request comment] How easily could an exploit be constructed based on the patch? Very difficult, requiring arranging an OOM to happen during JIT compilation. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? No. Which older supported branches are affected by this flaw? Everything back to FF 50. If not all supported branches, which bug introduced the flaw? Bug 1248163. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Should be trivial. How likely is this patch to cause regressions; how much testing does it need? Unlikely.
Attachment #8878468 -
Attachment is obsolete: true
Attachment #8878503 -
Flags: sec-approval?
Attachment #8878503 -
Flags: review+
Comment 9•7 years ago
|
||
sec-approval+ for checkin on June 26 or after. We'll want Beta and ESR52 patches nominated and approved to go in after that.
status-firefox54:
--- → wontfix
status-firefox56:
--- → affected
status-firefox-esr52:
--- → affected
tracking-firefox55:
--- → +
tracking-firefox56:
--- → +
tracking-firefox-esr52:
--- → 55+
Whiteboard: [jsbugmon:] → [jsbugmon:][checkin on 6/26]
Updated•7 years ago
|
Attachment #8878503 -
Flags: sec-approval? → sec-approval+
Comment 10•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e7414bbec32d3e075551485eb83580021aea70d8 This grafts cleanly to Beta and ESR52, so please nominate it when you get a chance. Sorry for adding the wrong reviewer to the commit message too, will fix on uplift :(
Flags: needinfo?(jcoppeard)
Whiteboard: [jsbugmon:][checkin on 6/26] → [jsbugmon:]
Comment 11•7 years ago
|
||
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/725d1be7b81d for https://treeherder.mozilla.org/logviewer.html#?job_id=109948501&repo=mozilla-inbound
Comment 12•7 years ago
|
||
Fixed and re-pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/d516c35eabf14738e03728f9be602b732e63078c
Flags: needinfo?(jcoppeard)
Comment 13•7 years ago
|
||
Updated patch.
Attachment #8878503 -
Attachment is obsolete: true
Attachment #8881465 -
Flags: review+
Comment 14•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d516c35eabf1
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Updated•7 years ago
|
Status: RESOLVED → VERIFIED
Comment 15•7 years ago
|
||
JSBugMon: This bug has been automatically verified fixed.
Updated•7 years ago
|
Group: javascript-core-security → core-security-release
Comment 16•7 years ago
|
||
Comment on attachment 8881465 [details] [diff] [review] bug1369994-typed-array-init v3 Approval Request Comment [Feature/Bug causing the regression]: Bug 1248163. [User impact if declined]: Possible crash / security vulnerability. [Is this code covered by automated tests?]: Yes. [Has the fix been verified in Nightly?]: Yes. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: It's a simple change to initialise a template object before a fallible operation to avoid exposing it to GC before it's been initialised. [String changes made/needed]: None.
Attachment #8881465 -
Flags: approval-mozilla-beta?
Comment 17•7 years ago
|
||
Comment on attachment 8881465 [details] [diff] [review] bug1369994-typed-array-init v3 [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: User impact if declined: sec-high bug. Fix Landed on Version: 56. Risk to taking this patch (and alternatives if risky): Low. String or UUID changes made by this patch: None.
Attachment #8881465 -
Flags: approval-mozilla-esr52?
Comment 18•7 years ago
|
||
Comment on attachment 8881465 [details] [diff] [review] bug1369994-typed-array-init v3 sec-high fix for beta55
Attachment #8881465 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 19•7 years ago
|
||
uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/0e5aa29c186d
Updated•7 years ago
|
Attachment #8881465 -
Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
Updated•7 years ago
|
Keywords: checkin-needed
Comment 20•7 years ago
|
||
https://hg.mozilla.org/releases/mozilla-esr52/rev/267b649087ff
Keywords: checkin-needed
Updated•7 years ago
|
Whiteboard: [jsbugmon:] → [jsbugmon:][adv-main55+][adv-esr52.3+]
Comment 21•7 years ago
|
||
(In reply to Jon Coppeard (:jonco) from comment #16) > [Is this code covered by automated tests?]: Yes. > [Has the fix been verified in Nightly?]: Yes. > [Needs manual test from QE? If yes, steps to reproduce]: No. Setting qe-verify- based on Jon Coppeard's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
Updated•7 years ago
|
Comment 22•7 years ago
|
||
JSBugMon: This bug has been automatically verified fixed on Fx55
Updated•6 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•