1518764 - Assertion failure: object->is<TypedArrayObject>() because of missing nullptr check after CheckedUnwrap

Status: RESOLVED FIXED

(Core :: JavaScript: Standard Library, defect)

Description

[André Bargull [:anba]]

var g = newGlobal();
var ta = new g.Int32Array(1);
Int32Array.prototype.filter.call(ta, function() {
    nukeAllCCWs();
    return true;
});

crashes with:

Assertion failure: object->is<TypedArrayObject>(), at /home/andre/git/mozilla-central/js/src/vm/SelfHosting.cpp:2111

Stacktrace:

#0  0x00005555572e5e24 in intrinsic_ConstructorForTypedArray(JSContext*, unsigned int, JS::Value*) (cx=0x7ffff5a18000, argc=1, vp=0x7ffff4cff288)
    at /home/andre/git/mozilla-central/js/src/vm/SelfHosting.cpp:2111
#1  0x0000555556e715d5 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (cx=0x7ffff5a18000, native=0x5555572e5c70 <intrinsic_ConstructorForTypedArray(JSContext*, unsigned int, JS::Value*)>, args=...) at /home/andre/git/mozilla-central/js/src/vm/Interpreter.cpp:443
#2  0x0000555556e5f4e0 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (cx=0x7ffff5a18000, args=..., construct=js::NO_CONSTRUCT)
    at /home/andre/git/mozilla-central/js/src/vm/Interpreter.cpp:535
#3  0x0000555556e5fb2b in InternalCall(JSContext*, js::AnyInvokeArgs const&) (cx=0x7ffff5a18000, args=...) at /home/andre/git/mozilla-central/js/src/vm/Interpreter.cpp:590
#4  0x0000555556e5f91d in js::CallFromStack(JSContext*, JS::CallArgs const&) (cx=0x7ffff5a18000, args=...) at /home/andre/git/mozilla-central/js/src/vm/Interpreter.cpp:594
#5  0x0000555556e537f7 in Interpret(JSContext*, js::RunState&) (cx=0x7ffff5a18000, state=...) at /home/andre/git/mozilla-central/js/src/vm/Interpreter.cpp:3320
#6  0x0000555556e489f4 in js::RunScript(JSContext*, js::RunState&) (cx=0x7ffff5a18000, state=...) at /home/andre/git/mozilla-central/js/src/vm/Interpreter.cpp:423
#7  0x0000555556e5f692 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (cx=0x7ffff5a18000, args=..., construct=js::NO_CONSTRUCT)
    at /home/andre/git/mozilla-central/js/src/vm/Interpreter.cpp:563
#8  0x0000555556e5fb2b in InternalCall(JSContext*, js::AnyInvokeArgs const&) (cx=0x7ffff5a18000, args=...) at /home/andre/git/mozilla-central/js/src/vm/Interpreter.cpp:590
#9  0x0000555556e5fba3 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) (cx=0x7ffff5a18000, fval=$JS::Value((JSObject *) 0x7ffff4fb7e70 [object Function "filter"]), thisv=$JS::Value((JSObject *) 0x7ffff7e00980 [object Proxy]), args=..., rval=$JS::Value((JSObject *) 0x7ffff4f8f180 [object Function "call"]))
    at /home/andre/git/mozilla-central/js/src/vm/Interpreter.cpp:606
#10 0x000055555716dc41 in js::fun_call(JSContext*, unsigned int, JS::Value*) (cx=0x7ffff5a18000, argc=2, vp=0x7ffff4cff0a0) at /home/andre/git/mozilla-central/js/src/vm/JSFunction.cpp:1192
...

Comment 1

[André Bargull [:anba]]

Hey Jan, do you think the s-s flag is justified here? This bug is caused by a missing nullptr check after [1], which means the protoKey computed in [2] has undefined contents when object is nullptr. But when I checked the value of protoKey in an opt build, it was always JSProto_Proxy, which is okay from a security POV and means the bug can be opened. I'm just not sure if it's always JSProto_Proxy under all compilers and compiler options, because dereferencing a nullptr is UB, so other compilers could potentially compute a different protoKey and because protoKey is later used as an index into the slots of the global object, it looks more serious if the content of protoKey may be a random value...

[1] https://searchfox.org/mozilla-central/rev/c3ebaf6de2d481c262c04bb9657eaf76bf47e2ac/js/src/vm/SelfHosting.cpp#2110
[2] https://searchfox.org/mozilla-central/rev/c3ebaf6de2d481c262c04bb9657eaf76bf47e2ac/js/src/vm/SelfHosting.cpp#2113

Comment 2

[Jan de Mooij [:jandem]]

(In reply to André Bargull [:anba] from comment #1)

This bug is caused by a missing nullptr check after [1],

|object| there will be a dead wrapper so CheckedUnwrap will just return it unchanged instead of nullptr?

Comment 3

[André Bargull [:anba]]

(In reply to Jan de Mooij [:jandem] from comment #2)

|object| there will be a dead wrapper so CheckedUnwrap will just return it unchanged instead of nullptr?

Oh, sure. I've misremembered how CheckedUnwrap works and thought it returns nullptr for dead wrappers. Sorry for the trouble!

Comment 4

[André Bargull [:anba]]

Attachment: bug1518764.patch

Switching to Jason's new UnwrapAndDowncastValue function should fix this issue quite easily.

Also: Can you unhide this bug given that it isn't actually s-s? Thanks! :-)

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 9035340 [details] [diff] [review]
bug1518764.patch

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

Great find, thanks!

Comment 6

[André Bargull [:anba]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d37d41cf36cd21d8c19b1a891d778eb647ba5f8b

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed68c008e5d8
Handle dead proxies in intrinsic_ConstructorForTypedArray by switching to UnwrapAndDowncastValue. r=jandem

Comment 8

[Cristina Coroiu [:ccoroiu]]

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