1798856 - Assertion failure: !val.isMagic() coming from Reflect.parse

Status: RESOLVED DUPLICATE of bug 1800629

(Core :: JavaScript Engine, defect, P3)

Description

[Samuel Groß]

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

function main() {
function v0(v1,v2) {
    class V3 {
        constructor(v5,v6,v7) {
        }
        fill(v9,v10,...v11) {
        }
    }
    with (v2) {
        const v12 = MAX_SAFE_INTEGER;
        async function* v13(v14,v15,v16) {
            const v18 = await v13;
            yield* `string${v15}bZXNNwokeO${v1}RFyGuoMK4k`;
            return v18;
        }
    }
    return v0;
}
const v20 = "string".split();
v0.classStatement = v0;
v0.builder = v0;
const v21 = {"builder":v20,"c":v0,"d":"string"};
const v23 = [-2147483647,-2147483647,-2147483647,-2147483647,-2147483647];
const v25 = Reflect.parse(v0,v0);
gc();
}
main();
// CRASH INFO
// ==========
// TERMSIG: 11
// STDERR:
// Assertion failure: !val.isMagic(), at /home/builder/firefox/js/src/vm/JSObject.cpp:2637
// #01: js::ToObjectSlow(JSContext*, JS::Handle<JS::Value>, bool)[./spidermonkey/js +0x1c2e7c0]
// #02: ???[./spidermonkey/js +0x192de70]
// #03: ???[./spidermonkey/js +0x19107fc]
// #04: ???[./spidermonkey/js +0x190e4c5]
// #05: ???[./spidermonkey/js +0x1928350]
// #06: ???[./spidermonkey/js +0x192a362]
// #07: ???[./spidermonkey/js +0x1d62f25]
// #08: ???[./spidermonkey/js +0x1d6afdb]
// #09: ???[./spidermonkey/js +0x1d6ac3b]
// #10: ???[./spidermonkey/js +0x1d5f718]
// #11: ???[./spidermonkey/js +0x1d4f862]
// #12: ???[./spidermonkey/js +0x1d61880]
// #13: ???[./spidermonkey/js +0x1d5fedb]
// #14: ???[./spidermonkey/js +0x1d4cabc]
// #15: ???[./spidermonkey/js +0x1d4c2c1]
// #16: ???[./spidermonkey/js +0x1d4ba4c]
// #17: ???[./spidermonkey/js +0x1d0fbb8]
// #18: ???[./spidermonkey/js +0x192904a]
// #19: ???[./spidermonkey/js +0x192848e]
// #20: ???[./spidermonkey/js +0x191b515]
// #21: ???[./spidermonkey/js +0x190e4c5]
// #22: ???[./spidermonkey/js +0x192c282]
// #23: ???[./spidermonkey/js +0x192c931]
// #24: ???[./spidermonkey/js +0x1aceee6]
// #25: JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>)[./spidermonkey/js +0x1acf1c0]
// #26: ???[./spidermonkey/js +0x18080f8]
// #27: ???[./spidermonkey/js +0x1801466]
// #28: ???[/lib/x86_64-linux-gnu/libc.so.6 +0x29d90]
// #29: __libc_start_main[/lib/x86_64-linux-gnu/libc.so.6 +0x29e40]
// #30: ???[./spidermonkey/js +0x17cbf29]
// #31: ??? (???:???)
// STDOUT:

The issue seems to be related to Reflect.parse which is non-standard and doesn't appear to be exposed in Firefox, so this issue may have no security impact in practice.

Comment 1

[Samuel Groß]

FWIW I have a number of other crashes related to Reflect.parse (which may or may not have the same root cause as this one) which I'm happy to share, but I first wanted to see if this feature should even be exposed to fuzzers?

Comment 2

[Andrew McCreight [:mccr8]]

Maybe Decoder has thoughts on whether it is worth fuzzing Reflect.parse.

Comment 3

[Ted Campbell [:tcampbell]]

Attachment: WIP: Bug 1798856 - Fix Reflect.parse builder support for classes

The class heritage is an optional node, so ensure we don't try to leak a
JS_SERIALIZE_NO_NODE value to script.

Comment 4

[Jan de Mooij [:jandem]]

(In reply to Samuel Groß from comment #1)

FWIW I have a number of other crashes related to Reflect.parse (which may or may not have the same root cause as this one) which I'm happy to share, but I first wanted to see if this feature should even be exposed to fuzzers?

As long as we have this feature, we should continue fuzzing it IMO. Reflect.parse is used by devtools in a few places according to Searchfox so removing it isn't completely trivial unfortunately.

Comment 5

[Andrew McCreight [:mccr8]]

I'll conservatively mark it sec-moderate if this feature is used by devtools, though it may not actually be exploitable.

Comment 6

[Matthew Gaudet (he/him) [:mgaudet]]

Arai: Would you be able to verify Ted's patch then land it?

Comment 7

[Tooru Fujisawa [:arai]]

the custom builder feature is removed from Reflect.parse by bug 1800629, and the affected code no longer exists.

Comment 8

[Matthew Gaudet (he/him) [:mgaudet]]

Thank you arai!

Comment 9

[Daniel Veditz [:dveditz]]

Sorry for the burst of bugspam: filter on tinkling-glitter-filtrate
Adding reporter-external keyword to security bugs found by non-employees for accounting reasons