Open Bug 2078778 Opened 1 month ago Updated 1 month ago

for-await loop rejects with awaited return() rejection instead of the loop body's exception (AsyncIteratorClose step 5) in BytecodeEmitter::emitIteratorCloseInScope

Categories

(Core :: JavaScript Engine, defect, P3)

defect

Tracking

()

UNCONFIRMED

People

(Reporter: bugmon, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: testcase)

Attachments

(4 files)

BytecodeEmitter::emitIteratorCloseInScope, for IteratorKind::Async with CompletionKind::Throw, opens its exception-swallowing try/catch while ITER is on the operand stack, so the catch try-note records a stack depth that includes ITER. Inside the try, the call to return() consumes ITER, and the Throw emitted by emitCheckAwaitResumeKind pops the awaited value before raising, so a rejection of the awaited return() result is raised one slot below the note's recorded depth. The try-note filters only accept notes with stackDepth <= the current depth, so the catch is skipped, the rejection propagates to the function's outer handler, and the ThrowWithStack that would rethrow the loop body's exception is never reached.

A web page script can make an async function reject with the wrong exception by throwing out of a for-await loop whose iterator's return() method returns a rejected promise. The demonstrated effect is a specification violation (ECMA-262 AsyncIteratorClose step 5): the original exception is replaced by the rejection reason of return(), in every execution tier tested. No crash, assertion or memory-safety effect was observed, and a return() that throws synchronously is handled correctly.

Build Info

Affected Code

File: js/src/frontend/BytecodeEmitter.cpp, line 3007-3017

  Maybe<TryEmitter> tryCatch;

  if (completionKind == CompletionKind::Throw) {
    tryCatch.emplace(this, TryEmitter::Kind::TryCatch,
                     TryEmitter::ControlKind::NonSyntactic);

    if (!tryCatch->emitTry()) {
      //            [stack] ... ITER
      return false;
    }
  }

File: js/src/frontend/BytecodeEmitter.cpp, line 3052-3079

  if (!emit1(JSOp::Swap)) {
    //              [stack] ... RET ITER
    return false;
  }

  if (!emitCall(getIterCallOp(JSOp::Call, selfHostedIter), 0)) {
    //              [stack] ... RESULT
    return false;
  }

  // 7.4.7 AsyncIteratorClose, step 5.d.
  if (iterKind == IteratorKind::Async) {
    ...
    if (!emitAwaitInScope(currentScope)) {
      //            [stack] ... RVAL? RESULT
      return false;
    }

File: js/src/frontend/BytecodeEmitter.cpp, line 3117-3132

  if (completionKind == CompletionKind::Throw) {
    if (!tryCatch->emitCatch()) {
      //            [stack] ... ITER EXC
      return false;
    }

    // Just ignore the exception thrown by call and await.
    if (!emit1(JSOp::Pop)) {
      //            [stack] ... ITER
      return false;
    }

    if (!tryCatch->emitEnd()) {
      //            [stack] ... ITER
      return false;
    }
  }

File: js/src/frontend/BytecodeEmitter.cpp, line 6713-6725

  const int32_t startDepth = bytecodeSection().stackDepth();
  //                [stack] RVAL RESUMEKIND

  InternalIfEmitter ifThrow(this);
  if (!ifThrow.emitThen()) {
    //              [stack] RVAL
    return false;
  }
  if (!emit1(JSOp::Throw)) {
    //              [stack]
    return false;
  }
  bytecodeSection().setStackDepth(startDepth - 1);

File: js/src/vm/Interpreter.cpp, line 1188-1197

class InterpreterTryNoteFilter {
  const InterpreterRegs& regs_;

 public:
  explicit InterpreterTryNoteFilter(const InterpreterRegs& regs)
      : regs_(regs) {}
  bool operator()(const TryNote* note) {
    return note->stackDepth <= regs_.stackDepth();
  }
};

File: js/src/vm/Interpreter.cpp, line 4037-4044

    CASE(Throw) {
      CHECK_BRANCH();
      ReservedRooted<Value> v(&rootValue0);
      POP_COPY_TO(v);
      MOZ_ALWAYS_FALSE(ThrowOperation(cx, v));
      /* let the code at error try to catch the exception. */
      goto error;
    }

File: [js/src/jit/JitFrames.cpp, line 190, 449-457](https://searchfox.org/firefox-main/rev/bf9757d91e2e53b8ab836f2a08bf4c0275fdcd23/js/src/jit/JitFrames.cpp#190, 449)

  bool operator()(const TryNote* note) { return note->stackDepth <= depth_; }

  bool operator()(const TryNote* note) {
    BaselineFrame* frame = frame_.baselineFrame();

    uint32_t numValueSlots = frame_.baselineFrameNumValueSlots();
    MOZ_RELEASE_ASSERT(numValueSlots >= frame->script()->nfixed());

    uint32_t currDepth = numValueSlots - frame->script()->nfixed();
    return note->stackDepth <= currDepth;
  }

The swallowing try is emitted with ITER on the stack; the call consumes ITER; the await's resume-kind check throws after popping RVAL. The dis() output of the testcase's async function shows the resulting try-note catch 6 208 300, while the Throw at pc 273 raises with five operand-stack slots live (NEXT ITER VALUE EXCEPTION STACK). All three try-note filters reject a note whose stackDepth exceeds the current depth, so the note is skipped.

Exploit Chain

  1. Script defines an async iterable whose next() resolves to { value, done: false } and whose return() returns a promise rejected with "inner".
  2. An async function iterates it with for await and the loop body throws "outer"; the loop's catch handler saves the exception and enters the async iterator-close sequence emitted by emitIteratorCloseInScope (CompletionKind::Throw).
  3. The close sequence calls return() (consuming ITER) and awaits the result; the await resumes with resume kind Throw and the Throw op pops RVAL and raises "inner" at operand-stack depth 5.
  4. The swallowing catch try-note has stackDepth 6, so the try-note filter (note->stackDepth <= current depth) skips it; the exception is caught by the function-level catch, which calls AsyncReject with "inner".
  5. The async function's promise rejects with "inner" instead of "outer"; ThrowWithStack for the saved original exception is never executed.

Steps to Reproduce

  1. Build the SpiderMonkey debug shell from the unmodified tree with /app/mozconfigs/mozconfig.js.linux.debug (binary: /firefox/obj-js-debug/dist/bin/js).
  2. Run testcase.js with js_shell_differential_evaluator: shell_a=/firefox/obj-js-debug/dist/bin/js (engine spidermonkey), shell_b=/app/tools/d8/d8 (engine v8), no extra flags. Result: differs: true, SpiderMonkey exit 3, d8 exit 0.
  3. Run again with flags_a=["--differential-testing"]. Result: differs: true, SpiderMonkey exit 3, d8 exit 0.
  4. No prefs, source patches, environment variables or extra shell flags are needed.

Security Impact

  • Severity: Low
  • Attacker capability: Script can cause an async function's promise to reject with the rejection reason of the async iterator's return() result instead of the exception thrown by the for-await loop body. This is a language-semantics (spec conformance) defect; no memory corruption, assertion failure or cross-tier divergence was observed.
  • Preconditions: None beyond running script: standard for-await syntax in an async function with an async iterator whose return() result rejects while the loop body completes with a throw.

ASAN Report

No sanitizer report: this is a js_differential finding (exit-status divergence). Verifier's own runs:

Run 1 (no --differential-testing): differs: true
  shell_a argv: ["/firefox/obj-js-debug/dist/bin/js","--fuzzing-safe","/tmp/fx_audit_jsdiff_7qdtmmcn/testcase.js"] exit_code: 3, succeeded: false
  shell_b argv: ["/app/tools/d8/d8","/tmp/fx_audit_jsdiff_7qdtmmcn/testcase.js"] exit_code: 0, succeeded: true
  shell_a stderr:
  Unhandled rejection: (new Error("for-await rejected with inner instead of outer", "/tmp/fx_audit_jsdiff_7qdtmmcn/testcase.js", 25))
  Stack:
    @/tmp/fx_audit_jsdiff_7qdtmmcn/testcase.js:25:13

Run 2 (--differential-testing on SpiderMonkey): differs: true
  shell_a argv: ["/firefox/obj-js-debug/dist/bin/js","--fuzzing-safe","--differential-testing","/tmp/fx_audit_jsdiff_kdorghw_/testcase.js"] exit_code: 3, succeeded: false
  shell_b argv: ["/app/tools/d8/d8","/tmp/fx_audit_jsdiff_kdorghw_/testcase.js"] exit_code: 0, succeeded: true
  shell_a stderr:
  Unhandled rejection: (new Error("for-await rejected with inner instead of outer", "/tmp/fx_audit_jsdiff_kdorghw_/testcase.js", 25))
  Stack:
    @/tmp/fx_audit_jsdiff_kdorghw_/testcase.js:25:1

Controls:
  Inverted check (throw if e === "outer"): SpiderMonkey exit 0, stdout "rejected with inner"; d8 exit 1, stdout "rejected with outer", "1 pending unhandled Promise rejection(s) detected."
  return() throws synchronously instead of returning a rejected promise: differs: false, both exit 0, both print "rejected with outer".
  SpiderMonkey --no-jit-backend vs --ion-eager --no-threads: differs: false, both exit 3, both print "rejected with inner".

dis() of the async function (debug shell), exception table:
  kind   stack  start  end
  catch      6    208  300
  catch      3    182  197
  for-of     2     92  325
  catch      0     19  363
  00273: Throw   # NEXT ITER VALUE EXCEPTION STACK   (depth 5 after popping RVAL; inside the depth-6 catch range 208-300)
Attached patch fix.patch — — Splinter Review
Attached file crash_stack.txt —
Attached file testcase.js —
Attached file README.txt —

This bug was filed automatically by an AI-powered security testing tool. The analysis, testcase, and any suggested patches were generated by an AI model and have NOT been verified by a human.

Please review all details carefully before triaging. The issue was confirmed by an independent verifier, but for non-ASAN issues, the verifier has been made less strict. Additionally, the root cause analysis and suggested fix may contain errors.

Group: core-security → javascript-core-security
Blocks: sm-security
Severity: -- → S3
Priority: -- → P3

Just a correctness bug.

Group: javascript-core-security
Keywords: sec-other
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: