Closed Bug 1800629 Opened 2 years ago Closed 2 years ago

Assertion failure: isObject() coming from Reflect.parse

Categories

(Core :: JavaScript Engine, task, P2)

task

Tracking

()

RESOLVED FIXED
111 Branch
Tracking Status
firefox111 --- fixed

People

(Reporter: saelo, Assigned: iain)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

The following sample triggers an assertion failure in debug builds of Spidermonkey at latest HEAD:

function main() {
function v0(v1,v2) {
    return v1;
}
function v3(v4,v5,v6,v7) {
    class V8 {
        constructor(v10,...v11) {
        }
        getUTCHours(v13,v14,v15,v16) {
        }
        indexOf(v18,v19,v20) {
        }
        imul(v22,v23) {
        }
    }
    Object.defineProperty(v4, "builder", { enumerable: true, value: v3 })
    return Reflect;
}
function v25() {
    return v0;
}
Object.defineProperty(v3, "identifier", { get: v25 })
const v26 = v3(v3);
const v27 = v26.parse(v3,v3);
gc();
}
main();
// CRASH INFO
// ==========
// TERMSIG: 11
// STDERR:
// Assertion failure: isObject(), at obj-fuzzbuild/dist/include/js/Value.h:935
// #01: JS::Value::toObject() const[obj-fuzzbuild/dist/bin/js +0x188189d]
// #02: ???[obj-fuzzbuild/dist/bin/js +0x1d9a012]
// #03: ???[obj-fuzzbuild/dist/bin/js +0x1d98b0b]
// #04: ???[obj-fuzzbuild/dist/bin/js +0x1d8f4a1]
// #05: ???[obj-fuzzbuild/dist/bin/js +0x1d8696a]
// #06: ???[obj-fuzzbuild/dist/bin/js +0x1d9817f]
// #07: ???[obj-fuzzbuild/dist/bin/js +0x1d88492]
// #08: ???[obj-fuzzbuild/dist/bin/js +0x1d9a4b0]
// #09: ???[obj-fuzzbuild/dist/bin/js +0x1d98b0b]
// #10: ???[obj-fuzzbuild/dist/bin/js +0x1d856ec]
// #11: ???[obj-fuzzbuild/dist/bin/js +0x1d84ef1]
// #12: ???[obj-fuzzbuild/dist/bin/js +0x1d8467c]
// #13: ???[obj-fuzzbuild/dist/bin/js +0x1d48335]
// #14: ???[obj-fuzzbuild/dist/bin/js +0x196529a]
// #15: ???[obj-fuzzbuild/dist/bin/js +0x19646de]
// #16: ???[obj-fuzzbuild/dist/bin/js +0x19576d5]
// #17: ???[obj-fuzzbuild/dist/bin/js +0x194a575]
// #18: ???[obj-fuzzbuild/dist/bin/js +0x19684d2]
// #19: ???[obj-fuzzbuild/dist/bin/js +0x1968b81]
// #20: ???[obj-fuzzbuild/dist/bin/js +0x1b07036]
// #21: JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>)[obj-fuzzbuild/dist/bin/js +0x1b07310]
// #22: ???[obj-fuzzbuild/dist/bin/js +0x18437c4]
// #23: ???[obj-fuzzbuild/dist/bin/js +0x183cb36]
// #24: ???[/lib/x86_64-linux-gnu/libc.so.6 +0x2920a]
// #25: __libc_start_main[/lib/x86_64-linux-gnu/libc.so.6 +0x292bc]
// #26: ???[obj-fuzzbuild/dist/bin/js +0x18074a9]
// #27: ??? (???:???)

The bug seems to be related to the non-standard Reflect.parse API, and so likely cannot be triggered from web content. The bug also reproduces with the patch from https://bugzilla.mozilla.org/show_bug.cgi?id=1798856 applied, so I assume this is a separate issue.

Group: core-security → javascript-core-security

Here's a reduced testcase:

function foo(...x) {}

var options = {
  builder: {
    get identifier() { return (x) => x }
  }
};

const v27 = Reflect.parse(foo, options);

It looks like the problem occurs because we specify a user-defined builder that returns unexpected values. (Expected values look like ["IdExpr", { name: name }]) User-defined builders were added in bug 569487; the documentation was on MDN and is now gone. The only place I can find where we define an identifier method on a builder is in non262/reflect-parse/alternateBuilder.js, which describes itself as "A simple proof-of-concept that the builder API can be used to generate other formats, such as JsonMLAst". I can't find any evidence that user-defined builders, with or without identifier, exist anywhere outside of our own tests.

We may not be able to remove Reflect.parse completely, but we can probably remove this unused builder stuff.

Blocks: sm-runtime
Severity: -- → S4
Priority: -- → P2

Reflect.parse isn't exposed to users, and there are no uses of the builder feature outside of tests, so this isn't security-sensitive. Opening it up and removing the builder code.

Group: javascript-core-security
Keywords: sec-moderate
Duplicate of this bug: 1810414
Assignee: nobody → iireland
Status: NEW → ASSIGNED
Pushed by iireland@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6b9c280f186b
Remove custom builders from Reflect.parse r=arai
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 111 Branch
Duplicate of this bug: 1798856
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: