1878378 - Assertion failure: isExceptionPending(), at vm/JSContext.cpp:1136

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: debug stack

let x = [];
Function.prototype.call.bind(Array.prototype.push)(x, this);
Function.prototype.call.bind(Array.prototype.push)(x, 1);
Function.prototype.call.bind(Array.prototype.push)(x, 1);
let y = Function.prototype.call.bind(Array.prototype.shift)(x);
Function.prototype.call.bind(Array.prototype.shift)(x);
y["interruptIf"].apply(y, x);
new ShadowRealm().evaluate("m");

(gdb) bt
#0  JSContext::getPendingException (this=0x7ffff662e100, rval=...) at /home/ubumain/trees/mozilla-central/js/src/vm/JSContext.cpp:1136
#1  0x0000555557517a2b in js::ReportPotentiallyDetailedMessage (cx=0x7ffff7a1ca60 <_IO_stdfile_2_lock>, cx@entry=0x7ffff662e100, detailedError=detailedError@entry=675, genericError=genericError@entry=674) at /home/ubumain/trees/mozilla-central/js/src/builtin/ShadowRealm.cpp:165
#2  0x0000555557565579 in PerformShadowRealmEval (cx=0x7ffff662e100, sourceText=..., callerRealm=0x7ffff6529400, evalRealm=<optimized out>, rval=...) at /home/ubumain/trees/mozilla-central/js/src/builtin/ShadowRealm.cpp:314
#3  ShadowRealm_evaluate (cx=cx@entry=0x7ffff662e100, argc=<optimized out>, vp=<optimized out>) at /home/ubumain/trees/mozilla-central/js/src/builtin/ShadowRealm.cpp:360
#4  0x00005555571c0d65 in CallJSNative (cx=cx@entry=0x7ffff662e100, native=native@entry=0x555557564b00 <ShadowRealm_evaluate(JSContext*, unsigned int, JS::Value*)>, reason=reason@entry=js::CallReason::Call, args=...) at /home/ubumain/trees/mozilla-central/js/src/vm/Interpreter.cpp:480
#5  0x0000555557197e6b in js::InternalCallOrConstruct (cx=0x7ffff662e100, args=..., construct=construct@entry=js::NO_CONSTRUCT, reason=js::CallReason::Call) at /home/ubumain/trees/mozilla-central/js/src/vm/Interpreter.cpp:574
#6  0x0000555557198ddd in InternalCall (cx=0x7ffff7a1ca60 <_IO_stdfile_2_lock>, cx@entry=0x7ffff662e100, args=..., reason=1488292272, reason@entry=js::CallReason::Call) at /home/ubumain/trees/mozilla-central/js/src/vm/Interpreter.cpp:641
#7  0x0000555557198d47 in js::CallFromStack (cx=0x7ffff7a1ca60 <_IO_stdfile_2_lock>, cx@entry=0x7ffff662e100, args=..., reason=4154570531, reason@entry=js::CallReason::Call) at /home/ubumain/trees/mozilla-central/js/src/vm/Interpreter.cpp:646
#8  0x0000555557d7640c in js::jit::DoCallFallback (cx=0x7ffff662e100, frame=0x7fffffffcd08, stub=0x7ffff645a2f0, argc=1, vp=0x7fffffffccb8, res=...) at /home/ubumain/trees/mozilla-central/js/src/jit/BaselineIC.cpp:1659
#9  0x00003a1e53d30a5f in ?? ()
/snip

Run with --fuzzing-safe --no-threads --ion-eager --enable-shadow-realms, compile with AR=ar sh ../configure --enable-debug --enable-debug-symbols --with-ccache --enable-nspr-build --enable-ctypes --enable-gczeal --enable-rust-simd --disable-tests, tested on m-c rev 90d84a362f05.

Setting s-s to be safe. Also trying to get a better bisection range. m-c rev de7cd72a119c7301d185c882739ac3f6c6822378 (late-June 2022) added --enable-shadow-realms, and this still occurs as of m-c rev e963fffcb3a0 (late-June 2023).

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/356aa1ad2aa7
user: André Bargull
date: Mon Oct 31 13:32:56 2022 +0000
summary: Bug 1797750 - Part 3: Prefer internal API instead of the public JS API. r=mgaudet

:anba, is bug 1797750 a likely regressor?

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1797750

Comment 3

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

Definitely not Anba; this looks to be a straightforward implementation oversight in ShadowRealms.

Comment 4

[André Bargull [:anba]]

(In reply to Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) from comment #1)

:anba, is bug 1797750 a likely regressor?

Yes. Here and here, getPendingException needs to be guarded by isExceptionPending() to handle interrupt requests. (Before bug 1797750, JS_GetPendingException was called, which includes the check for isExceptionPending().)

Simpler test case (only needs --enable-shadow-realms):

// Request interrupt from shadow realm. Includes loop to ensure interrupt
// callback is called even when --ion-eager is used.
new ShadowRealm().evaluate("(interruptIf => { interruptIf(1); for (let i = 0; i < 10; ++i) {} })")(interruptIf);

Comment 5

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

Hah. I was trying to reduce this and getting more confused over time :) Did you want to write the patch?

Comment 6

[André Bargull [:anba]]

Sure, I can write a patch, but I won't get to it before next week, if that's okay.

Comment 7

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

Fine by me; Shadow Realms is off by default so this will have extremely low impact.

Comment 8

[André Bargull [:anba]]

Attachment: Bug 1878378: Handle non-catchable exceptions in shadow realm. r=mgaudet!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b0e7478a155e
Handle non-catchable exceptions in shadow realm. r=mgaudet

Comment 10

[Ryan VanderMeulen [:RyanVM]]

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

Comment 11

[Daniel Veditz [:dveditz]]

(In reply to Matthew Gaudet (he/him) [:mgaudet] from comment #7)

Fine by me; Shadow Realms is off by default so this will have extremely low impact.

For bug bounty purposes, if this hadn't been found and fixed what would the eventual impact be if we shipped with this bug?

Comment 12

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

I believe all that would have happened is we'd be throwing undefined rather than any exception type. Incorrect program execution, nothing exploitable methinks (I had actually been thinking we should unrestrict this)