Closed Bug 1437507 Opened 2 years ago Closed 2 years ago

Assertion failure: !shape->inDictionary(), at js/src/vm/Shape.cpp:271


(Core :: JavaScript Engine, defect, critical)

Not set



Tracking Status
firefox-esr52 59+ fixed
firefox58 --- wontfix
firefox59 + verified
firefox60 + verified


(Reporter: decoder, Assigned: bhackett)


(Blocks 1 open bug)


(5 keywords, Whiteboard: [jsbugmon:update][adv-main59+][adv-esr52.7+])


(1 file)

The following testcase crashes on mozilla-central revision 2b7d42d527af (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off --ion-eager):

function f(s) {
    var obj = {
        m: function() {}
    return obj;
var obj = f(2);
for (var i = 0; i < 1000; i++)
    obj[Symbol.for("x" + i)] = 1;


received signal SIGSEGV, Segmentation fault.
0x0000000000c2c178 in js::Shape::replaceLastProperty (cx=<optimized out>, base=..., proto=..., shape=..., shape@entry=...) at js/src/vm/Shape.cpp:271
#0  0x0000000000c2c178 in js::Shape::replaceLastProperty (cx=<optimized out>, base=..., proto=..., shape=..., shape@entry=...) at js/src/vm/Shape.cpp:271
#1  0x0000000000c2c222 in js::Shape::setObjectFlags (cx=cx@entry=0x7ffff5f16000, flags=flags@entry=js::BaseShape::DELEGATE, proto=..., last=0x7ffff4ce7588) at js/src/vm/Shape.cpp:1390
#2  0x0000000000c2c2f6 in JSObject::setFlags (cx=0x7ffff5f16000, obj=obj@entry=..., flags=flags@entry=js::BaseShape::DELEGATE, generateShape=generateShape@entry=JSObject::GENERATE_SHAPE) at js/src/vm/Shape.cpp:1352
#3  0x0000000000bdaec5 in JSObject::setDelegate (obj=..., cx=<optimized out>) at js/src/jsobj.h:193
#4  js::ObjectGroup::defaultNewGroup (cx=0x7ffff5f16000, clasp=clasp@entry=0x1fde8c0 <js::PlainObject::class_>, proto=..., associated=associated@entry=0x0) at js/src/vm/ObjectGroup.cpp:530
#5  0x0000000000a56533 in js::NewObjectWithGivenTaggedProto (cx=cx@entry=0x7ffff5f16000, clasp=clasp@entry=0x1fde8c0 <js::PlainObject::class_>, proto=..., allocKind=js::gc::AllocKind::OBJECT4_BACKGROUND, allocKind@entry=js::gc::AllocKind::OBJECT4, newKind=newKind@entry=js::TenuredObject, initialShapeFlags=initialShapeFlags@entry=0) at js/src/jsobj.cpp:789
#6  0x000000000058f04f in js::NewObjectWithGivenProto<js::PlainObject> (newKind=js::TenuredObject, allocKind=js::gc::AllocKind::OBJECT4, proto=..., cx=0x7ffff5f16000) at js/src/jsobjinlines.h:664
#7  js::ObjectCreateImpl (cx=0x7ffff5f16000, proto=..., proto@entry=..., newKind=newKind@entry=js::TenuredObject, group=..., group@entry=...) at js/src/builtin/Object.cpp:979
#8  0x0000000000652e72 in js::jit::GetTemplateObjectForNative (skipAttach=<synthetic pointer>, res=..., args=..., target=..., cx=0x7ffff5f16000) at js/src/jit/BaselineIC.cpp:1892
#9  js::jit::TryAttachCallStub (cx=0x7ffff5f16000, stub=0x7ffff49305e0, script=script@entry=..., pc=pc@entry=0x7ffff48129b4 ":\001", op=op@entry=JSOP_CALL, argc=argc@entry=1, vp=0x7fffffffcae8, constructing=false, isSpread=false, createSingleton=false, handled=0x7fffffffc5ff) at js/src/jit/BaselineIC.cpp:2191
#10 0x0000000000654dd5 in js::jit::DoCallFallback (cx=0x7ffff5f16000, frame=0x7fffffffcb48, stub_=<optimized out>, argc=<optimized out>, vp=0x7fffffffcae8, res=...) at js/src/jit/BaselineIC.cpp:2352
#11 0x000009eb160b694f in ?? ()
#35 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x7ffff4ce7588	140737300559240
rcx	0x7ffff6c282ad	140737333330605
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffbf50	140737488338768
rsp	0x7fffffffbea0	140737488338592
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4780	140737354024832
r10	0x58	88
r11	0x7ffff6b9e7a0	140737332766624
r12	0x7fffffffbf90	140737488338832
r13	0x7fffffffbff0	140737488338928
r14	0x8	8
r15	0x1	1
rip	0xc2c178 <js::Shape::replaceLastProperty(JSContext*, js::StackBaseShape&, js::TaggedProto, JS::Handle<js::Shape*>)+392>
=> 0xc2c178 <js::Shape::replaceLastProperty(JSContext*, js::StackBaseShape&, js::TaggedProto, JS::Handle<js::Shape*>)+392>:	movl   $0x0,0x0
   0xc2c183 <js::Shape::replaceLastProperty(JSContext*, js::StackBaseShape&, js::TaggedProto, JS::Handle<js::Shape*>)+403>:	ud2

Marking s-s because :jandem mentioned this could be security-related.
Bisection might be interesting...
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:
user:        Brian Hackett
date:        Wed May 13 07:17:53 2015 -0600
summary:     Bug 1162199 - Use unboxed objects by default, r=jandem.

This iteration took 170.785 seconds to run.
Looks like I found an ancient bug ^.^
Flags: needinfo?(bhackett1024)
How serious is this assertion? is the error condition handled or do we go off the rails?
Flags: needinfo?(jdemooij)
Attached patch patchSplinter Review
Yeah, this is a very old bug.  It's pretty serious too; in a non-DEBUG build we end up with an invalid shape that thinks it is in a dictionary but has a hashtable pointer as its child instead of a shape pointer like it should.  I haven't gotten this to do any bad behavior other than crashing at null but there is definitely the potential for corrupt accesses.
Assignee: nobody → bhackett1024
Flags: needinfo?(bhackett1024)
Attachment #8952778 - Flags: review?(jdemooij)
Keywords: sec-high
Flags: needinfo?(jdemooij)
Attachment #8952778 - Flags: review?(jdemooij) → review+
Comment on attachment 8952778 [details] [diff] [review]

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

Not easily.

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


Which older supported branches are affected by this flaw?


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

Bug 1162199.

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

Backports should be simple.

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

Very unlikely.
Attachment #8952778 - Flags: sec-approval?
sec-approval+ for trunk.
Please make and nominate Beta and ESR52 patches as well, to land after trunk.
Attachment #8952778 - Flags: sec-approval? → sec-approval+
Comment on attachment 8952778 [details] [diff] [review]

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: n/a
User impact if declined: security bug
Fix Landed on Version: not landed yet
Risk to taking this patch (and alternatives if risky): none
String or UUID changes made by this patch: none

See for more info.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1162199
[User impact if declined]: security bug
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: no
[Is the change risky?]: no
[Why is the change risky/not risky?]: simple tweak to some shape code
[String changes made/needed]: none
Attachment #8952778 - Flags: approval-mozilla-esr52?
Attachment #8952778 - Flags: approval-mozilla-beta?
Comment on attachment 8952778 [details] [diff] [review]

Simple sec-high fix, taking for 59b13 & 52.7.0.
Attachment #8952778 - Flags: approval-mozilla-esr52?
Attachment #8952778 - Flags: approval-mozilla-esr52+
Attachment #8952778 - Flags: approval-mozilla-beta?
Attachment #8952778 - Flags: approval-mozilla-beta+
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
JSBugMon: This bug has been automatically verified fixed.
Group: javascript-core-security → core-security-release
Whiteboard: [jsbugmon:update] → [jsbugmon:update][adv-main59+][adv-esr52.7+]
JSBugMon: This bug has been automatically verified fixed on Fx59
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.