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

RESOLVED FIXED in Firefox 58

Status

()

defect
P2
critical
RESOLVED FIXED
2 years ago
Last year

People

(Reporter: anba, Assigned: anba)

Tracking

({sec-other})

Trunk
mozilla58
Points:
---
Bug Flags:
in-testsuite +
qe-verify -

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox56 unaffected, firefox57 wontfix, firefox58 fixed)

Details

(Whiteboard: [adv-main58-][post-critsmash-triage])

Attachments

(1 attachment)

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
....
---
Group: core-security → javascript-core-security
Priority: -- → P2
Andre: what are the security implications of this?

Presumably affects 57 because the original bug was uplifted?
Flags: needinfo?(andrebargull)
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.
Flags: needinfo?(andrebargull)
Might as well just add a patch for this bug.
Assignee: nobody → andrebargull
Status: NEW → ASSIGNED
Attachment #8917952 - Flags: review?(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.
Attachment #8917952 - Flags: review?(till) → review+
Keywords: sec-other
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/56a94fa75d3f

Is there a user impact that justifies uplift consideration or can it ride the 58 train?
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Flags: needinfo?(andrebargull)
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
(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.
Flags: needinfo?(andrebargull)
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.
Group: javascript-core-security → core-security-release
Whiteboard: [adv-main58-]
Flags: qe-verify-
Whiteboard: [adv-main58-] → [adv-main58-][post-critsmash-triage]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.