Closed Bug 1475669 Opened 2 years ago Closed 2 years ago

Assertion failure: this->is<T>() with Debugger "promiseDependentPromises"

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox-esr60 --- wontfix
firefox62 --- wontfix
firefox63 --- wontfix
firefox64 --- fixed

People

(Reporter: anba, Assigned: jorendorff)

Details

(Keywords: sec-low, Whiteboard: [post-critsmash-triage][adv-main64+])

Attachments

(1 file)

Probably not S-S, because the Debugger is involved, but Jason offered to take a look and decide on it. :-)


Test case:
---
var g = newGlobal();
var dbg = new Debugger();
var gw = dbg.addDebuggee(g);

g.P = Promise;

g.eval(`
    function newPromiseCapability() {
        var resolve, reject, promise = new Promise(function(r1, r2) {
            resolve = r1;
            reject = r2;
        });
        return {promise, resolve, reject};
    }

    var {promise} = newPromiseCapability();

    P.prototype.then.call(promise);
`);

var promise = gw.makeDebuggeeValue(g.promise);

print(promise.promiseDependentPromises);
---


Asserts with:
---
Assertion failure: this->is<T>(), at /home/andre/git/mozilla-central/js/src/vm/JSObject.h:531

Thread 1 "mozjs-debug" received signal SIGSEGV, Segmentation fault.
0x00000000004acbca in JSObject::as<js::NativeObject> (this=(JSObject *) 0x7ffff7e00bf0 [object Proxy]) at /home/andre/git/mozilla-central/js/src/vm/JSObject.h:531
531             MOZ_ASSERT(this->is<T>());
---
Group: core-security → javascript-core-security
calling it sec-other while you look at it. If it's not a security bug please unhide it or ping us on #security to do so. If it is please take a stab at a severity rating and replace the sec-other.
Flags: needinfo?(jorendorff)
Keywords: sec-other
Sigh. Predictable. This is sec-low; it's a small part of an exploit, and even if you could fill in the blanks the exploit would have to involve convincing the user to open the debugger.

Easy to fix, just takes time...
Keywords: sec-othersec-low
Priority: -- → P1
Attached patch bug1475669.patchSplinter Review
Unwrap wrappers per |AddPromiseReaction()| [1], including its MOZ_RELEASE_ASSERTs for good measure.

[1] https://searchfox.org/mozilla-central/rev/88199de427d3c5762b7f3c2a4860c10734abd867/js/src/builtin/Promise.cpp#3731
Attachment #8993483 - Flags: review?(jorendorff)
Jason, when would you be able to review André's patch?
Comment on attachment 8993483 [details] [diff] [review]
bug1475669.patch

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

This returns an array of pointers to things in any number of different compartments. That's so strange that the name of the method should point it out.

(I don't know if the Debugger should return dependent promises from non-debuggee compartments or not; I'll check. Separate bug if not.)
Attachment #8993483 - Flags: review?(jorendorff) → review+
Also, this needs a test. Taking.
Assignee: nobody → jorendorff
Flags: needinfo?(jorendorff)
https://hg.mozilla.org/mozilla-central/rev/cb2f928fff56
Group: javascript-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main64+]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.