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

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

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);
f();
for (var i = 0; i < 1000; i++)
    obj[Symbol.for("x" + i)] = 1;
Object.getOwnPropertySymbols(Object.create(obj)).length;


Backtrace:

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.

Comment 1

[Jan de Mooij [:jandem]]

Bisection might be interesting...

Comment 2

[Fuzzing Team]

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/89c05305c708
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.

Comment 3

[Christian Holler (:decoder)]

Looks like I found an ancient bug ^.^

Comment 4

[Daniel Veditz [:dveditz]]

How serious is this assertion? is the error condition handled or do we go off the rails?

Comment 5

[Brian Hackett [Laid off!]]

Attachment: patch

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.

Comment 6

[Brian Hackett [Laid off!]]

Comment on attachment 8952778 [details] [diff] [review]
patch

[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?

No.

Which older supported branches are affected by this flaw?

All.

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.

Comment 7

[Al Billings [:abillings - ex-MoCo]]

sec-approval+ for trunk.
Please make and nominate Beta and ESR52 patches as well, to land after trunk.

Comment 8

[Brian Hackett [Laid off!]]

Comment on attachment 8952778 [details] [diff] [review]
patch

[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 https://wiki.mozilla.org/Release_Management/ESR_Landing_Process 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

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ca6b74831ec3db204e024b07f200b0d1ce93557e

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8952778 [details] [diff] [review]
patch

Simple sec-high fix, taking for 59b13 & 52.7.0.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/9d7c295d9570

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/ca6b74831ec3

Comment 13

[Fuzzing Team]

JSBugMon: This bug has been automatically verified fixed.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/5b3a5de48912

Comment 15

[Fuzzing Team]

JSBugMon: This bug has been automatically verified fixed on Fx59