1799824 - Assertion failure: idx < getDenseInitializedLength() in ShortestPaths() in TestingFunctions.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P1)

Description

[Samuel Groß]

The following sample causes an assertion failure in debug builds of Spidermonkey at current HEAD:

function main() {
const v0 = [];
v0.isConcatSpreadable = v0;
const v1 = v0.isConcatSpreadable;
const v2 = [v1];
function v3() {
    const v5 = [Reflect,Reflect,Reflect,Reflect];
    const v6 = v0.shift;
    const v7 = Reflect.apply(v6,v2,v5);
    return v2;
}
function v8(v9) {
    return v0;
}
Object.defineProperty(v2, "start", { get: v3, set: v8 })
const v11 = this.shortestPaths(v2,v2);
gc();
}
main();
// CRASH INFO
// ==========
// TERMSIG: 11
// STDERR:
// Assertion failure: idx < getDenseInitializedLength(), at /home/builder/firefox/js/src/vm/NativeObject.h:597
// #01: ???[./spidermonkey/js +0x20056fa]
// #02: ???[./spidermonkey/js +0x193614a]
// #03: ???[./spidermonkey/js +0x193540b]
// #04: ???[./spidermonkey/js +0x25c1ead]
// #05: ??? (???:???)
// STDOUT:
// ARGS: ./spidermonkey/js --baseline-warmup-threshold=10 --ion-warmup-threshold=100 --ion-check-range-analysis --ion-extra-checks --fuzzing-safe --disable-oom-functions --reprl

The issue here seems to be that the "start" getter resizes the array in v2, which is not expected by the shortestPaths function and therefore leads to this assertion failure. As the shortestPaths function is, from what I understand, only used for testing and is only available in the Spidermonkey shell, this bug probably has no security impact in practice. As such, please adjust the security labels accordingly once you have verified that this is in fact the case.

Comment 1

[Daniel Veditz [:dveditz]]

.shortestPaths() is not exposed to content, but it does appear to be used by DevTools code in the Heap Snapshot feature. They probably aren't torturing it the way you are :-) but maybe you could create a malicious page "just in case" someone tries to debug it?

Conservatively we can start rating this "moderate" because the potential impact of a bounds issue, but it seems really unlikely to be triggerable.

Comment 2

[Christian Holler (:decoder)]

The failure appears to be in the ShortestPaths testing function itself (the native function in js/src/builtin/TestingFunctions.cpp), not the functionality exposed to DevTools. This native function appears to be shell-only and is not referenced anywhere else.

Comment 3

[Jan de Mooij [:jandem]]

Yes this is a problem with just the testing function.

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1799824 part 1 - Check array elements after potential side-effects in shortestPaths testing function. r?sfink!

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Bug 1799824 part 2 - Support more GC things in shortestPaths testing function. r?sfink!

While we're here, it's easy to add support for other GC things like BigInt.

Depends on D161767

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9b5563580557
part 1 - Check array elements after potential side-effects in shortestPaths testing function. r=sfink
https://hg.mozilla.org/integration/autoland/rev/3cbb5b420782
part 2 - Support more GC things in shortestPaths testing function. r=sfink

Comment 7

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/9b5563580557
https://hg.mozilla.org/mozilla-central/rev/3cbb5b420782