André Bargull [:anba] Reporter
	Description • 6 years ago

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>());
---

	Daniel Veditz [:dveditz]
	Comment 1 • 6 years ago

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.

	Jason Orendorff [:jorendorff] Assignee
	Comment 2 • 6 years ago

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...

	André Bargull [:anba] Reporter
	Comment 3 • 6 years ago

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

	Nicolas B. Pierron [:nbp]
	Comment 4 • 6 years ago

Jason, when would you be able to review André's patch?

	Jason Orendorff [:jorendorff] Assignee
	Comment 5 • 6 years ago

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.)

	Jason Orendorff [:jorendorff] Assignee
	Comment 6 • 6 years ago

Also, this needs a test. Taking.

	Jason Orendorff [:jorendorff] Assignee
	Comment 7 • 6 years ago

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

	Jason Orendorff [:jorendorff] Assignee
	Comment 8 • 6 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/cb2f928fff562ff38b66f474ab8acd628535d985
Bug 1475669 - Return proper result for wrapped reactions. r=jorendorff

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 9 • 6 years ago

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