Missing realm checks in Array and Promise functions

RESOLVED FIXED in Firefox 66

Status

()

defect
RESOLVED FIXED
5 months ago
4 months ago

People

(Reporter: anba, Assigned: jandem)

Tracking

Trunk
mozilla66
Points:
---

Firefox Tracking Flags

(firefox66 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 months ago
Expected: All tests pass.
Actual: All three tests fail

Tests:
---
var g = newGlobal({sameCompartmentAs: this});

function testArrayOf() {
    var a = Array.of.call(g.Array);
    assertEq(a instanceof g.Array, true);
}
testArrayOf();

function testPromiseThen() {
    var p = Promise.resolve(0);
    p.constructor = g.Promise;
    var r = p.then(() => {});
    assertEq(r instanceof g.Promise, true);
}
testPromiseThen();

function testPromiseCatch() {
    Boolean.prototype.then = g.Promise.prototype.then;
    try {
        var r = Promise.prototype.catch.call(false);
    } catch (e) {
        assertEq(e instanceof g.TypeError, true);
    }
}
testPromiseCatch();
---
(Assignee)

Comment 1

5 months ago
Can you be more specific about where the problem is? :)

With the patches for bug 1514210 I have a browser test failure that's related to using the wrong global somewhere in promise code I think, but I've no idea where to start..
Flags: needinfo?(andrebargull)
(Reporter)

Comment 2

5 months ago
> Can you be more specific about where the problem is? :)

Sure. All three cases are related to missing realm checks for callers to IsNativeFunction.


Array.of:
The IsArrayConstructor at [1] also returns |true| for cross-realm Array constructors. Since I don't think it's likely to call Array.of with |this| set to a cross-realm Array constructor, it should be okay to simply add a realm check and fall into the slow path when |this| is an Array constructor from a different realm.

[1] https://searchfox.org/mozilla-central/rev/2e5e28f518524f8af5b158ddb605022b6a2d68cf/js/src/builtin/Array.cpp#3679


Promise.prototype.then:
The IsNativeFunction check at [2] ignores the case when |cVal| is a cross-realm Promise constructor. I can't tell offhand how likely cross-realm constructors are in this case.

[2] https://searchfox.org/mozilla-central/rev/2e5e28f518524f8af5b158ddb605022b6a2d68cf/js/src/builtin/Promise.cpp#1231


Promise.prototype.catch:
Also a missing realm check for a call to IsNativeFunction [3].

[3] https://searchfox.org/mozilla-central/rev/2e5e28f518524f8af5b158ddb605022b6a2d68cf/js/src/builtin/Promise.cpp#4041
Flags: needinfo?(andrebargull)
(Assignee)

Comment 3

5 months ago
Thank you! That helps a lot :)
(Assignee)

Comment 4

4 months ago

Hopefully when we enable same-compartment realms for content globals, jsreftests running in the browser will use it for globals they create. We would then test the same-compartment case there and the cross-compartment case in the shell.

(Assignee)

Updated

4 months ago
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED

Comment 6

4 months ago
Pushed by jdemooij@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9bab87de8e3c
Add missing realm checks to some Array and Promise functions. r=anba

Comment 7

4 months ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
You need to log in before you can comment on or make changes to this bug.