1406463 - Assertion failure: this->is<T>(), at /home/andre/hg/mozilla-inbound/js/src/jsobj.h:531

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[André Bargull [:anba]]

Follow-up from bug 1347984:
---
var P = newGlobal().eval(`
(class extends Promise {
    static resolve(o) {
        return o;
    }
});
`);

Promise.all.call(P, [{
    then(r) {
        nukeAllCCWs();
        r();
    }
}]);
---


Asserts with:
---
Thread 1 "js" received signal SIGSEGV, Segmentation fault.
0x0000000000524bcc in JSObject::as<js::NativeObject> (this=<optimised out>) at /home/andre/hg/mozilla-inbound/js/src/jsobj.h:531
531             MOZ_ASSERT(this->is<T>());
---


Stack trace:
---
#0  0x0000000000524bcc in JSObject::as<js::NativeObject>() (this=<optimised out>) at /home/andre/hg/mozilla-inbound/js/src/jsobj.h:531
#1  0x00000000005e9aec in PromiseAllResolveElementFunction(JSContext*, unsigned int, JS::Value*) (cx=0x7ffff6976000, argc=<optimised out>, vp=<optimised out>)
    at /home/andre/hg/mozilla-inbound/js/src/builtin/Promise.cpp:2113
#2  0x000000000055d181 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (cx=0x7ffff6976000, native=0x5e98e0 <PromiseAllResolveElementFunction(JSContext*, unsigned int, JS::Value*)>, args=...) at /home/andre/hg/mozilla-inbound/js/src/jscntxtinlines.h:293
#3  0x0000000000551a2f in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (cx=cx@entry=0x7ffff6976000, args=..., construct=construct@entry=js::NO_CONSTRUCT)
    at /home/andre/hg/mozilla-inbound/js/src/vm/Interpreter.cpp:495
#4  0x0000000000551ecd in InternalCall(JSContext*, js::AnyInvokeArgs const&) (cx=0x7ffff6976000, args=...) at /home/andre/hg/mozilla-inbound/js/src/vm/Interpreter.cpp:540
#5  0x0000000000543fac in Interpret(JSContext*, js::RunState&) (args=..., cx=<optimised out>) at /home/andre/hg/mozilla-inbound/js/src/vm/Interpreter.cpp:546
#6  0x0000000000543fac in Interpret(JSContext*, js::RunState&) (cx=0x7ffff6976000, state=...) at /home/andre/hg/mozilla-inbound/js/src/vm/Interpreter.cpp:3085
#7  0x0000000000551491 in js::RunScript(JSContext*, js::RunState&) (cx=0x7ffff6976000, state=...) at /home/andre/hg/mozilla-inbound/js/src/vm/Interpreter.cpp:435
#8  0x0000000000551b8f in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (cx=cx@entry=0x7ffff6976000, args=..., construct=construct@entry=js::NO_CONSTRUCT)
    at /home/andre/hg/mozilla-inbound/js/src/vm/Interpreter.cpp:513
#9  0x0000000000551ecd in InternalCall(JSContext*, js::AnyInvokeArgs const&) (cx=cx@entry=0x7ffff6976000, args=...) at /home/andre/hg/mozilla-inbound/js/src/vm/Interpreter.cpp:540
#10 0x0000000000552030 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) (cx=cx@entry=0x7ffff6976000, fval=..., 
    fval@entry=$jsval((JSObject *) 0x7ffff44b5420 [object Function "then"]), thisv=..., thisv@entry=$jsval((JSObject *) 0x7ffff4491190 [object Object]), args=..., rval=JSVAL_VOID)
    at /home/andre/hg/mozilla-inbound/js/src/vm/Interpreter.cpp:559
#11 0x00000000005e3965 in BlockOnPromise(JSContext*, JS::HandleValue, JS::HandleObject, JS::HandleValue, JS::HandleValue) (rval=..., arg1=$jsval((JSObject *) 0x7ffff7e00a00 [object Proxy]), arg0=$jsval((JSObject *) 0x7ffff7e00b60 [object Function <unnamed>]), thisv=..., fval=..., cx=0x7ffff6976000) at /home/andre/hg/mozilla-inbound/js/src/vm/Interpreter.h:122
#12 0x00000000005e3965 in BlockOnPromise(JSContext*, JS::HandleValue, JS::HandleObject, JS::HandleValue, JS::HandleValue) (cx=0x7ffff6976000, promiseVal=..., 
    promiseVal@entry=$jsval((JSObject *) 0x7ffff4491190 [object Object]), blockedPromise_=(JSObject * const) 0x7ffff7e00a40 [object Proxy], onFulfilled=..., 
    onFulfilled@entry=$jsval((JSObject *) 0x7ffff7e00b60 [object Function <unnamed>]), onRejected=..., onRejected@entry=$jsval((JSObject *) 0x7ffff7e00a00 [object Proxy]))
    at /home/andre/hg/mozilla-inbound/js/src/builtin/Promise.cpp:3128
#13 0x00000000005e9450 in Promise_static_all(JSContext*, unsigned int, JS::Value*) (done=0x7fffffffc01f, reject=(JSObject * const) 0x7ffff7e00a00 [object Proxy], resolve=..., promiseObj=(JSObject * const) 0x7ffff7e00a40 [object Proxy], C=(JSObject * const) 0x7ffff449f100 [object Proxy], iterator=..., cx=0x7ffff6976000) at /home/andre/hg/mozilla-inbound/js/src/builtin/Promise.cpp:2065
#14 0x00000000005e9450 in Promise_static_all(JSContext*, unsigned int, JS::Value*) (cx=0x7ffff6976000, argc=<optimised out>, vp=<optimised out>)
    at /home/andre/hg/mozilla-inbound/js/src/builtin/Promise.cpp:1735
....
---

Comment 1

[Daniel Veditz [:dveditz]]

Andre: what are the security implications of this?

Presumably affects 57 because the original bug was uplifted?

Comment 2

[André Bargull [:anba]]

From the looks of it, it should be quite similar to bug 1347984. We're effectively invoking a NativeObject elements method on a DeadObjectProxy and to repeat :jandem's words from 1347984:
> [...] we fell through to the array case and there things go horribly wrong.

So we will definitely crash, but it's probably unlikely that the crash can be used as an exploit. I've only hidden this bug because I think it's better to be safe than sorry.

Comment 3

[André Bargull [:anba]]

Attachment: bug1406463.patch

Might as well just add a patch for this bug.

Comment 4

[Till Schneidereit [:till]]

Comment on attachment 8917952 [details] [diff] [review]
bug1406463.patch

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

Thank you for fixing this, and sorry for the review delay.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/56a94fa75d3fbf7638277e0c0c4d2a020ebfb09a

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/56a94fa75d3f

Is there a user impact that justifies uplift consideration or can it ride the 58 train?

Comment 7

[André Bargull [:anba]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #6)
> https://hg.mozilla.org/mozilla-central/rev/56a94fa75d3f
> 
> Is there a user impact that justifies uplift consideration or can it ride
> the 58 train?

The bug can crash Firefox, but in comparison to bug 1347984, I don't think it's possible to hit this bug when you're just browsing, because the set-up is more involved and requires (ab)using certain extension points in ES6 Promises (a Promise subclass needs to be constructed whose static `resolve` method was replaced - and this is not a thing which occurs in normal web sites). That being said, the actual fix is super-safe to uplift, so there's no risk when we uplift it now instead of letting it ride the trains.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Thanks for the reply. We're trying to keep uplifts to those that are high-impact at this point in the cycle, so I'm going to call 57 wontfix.