Closed Bug 2029451 Opened 7 months ago Closed 4 months ago

Heap corruption via concurrent getc_unlocked(stdin) in [@ js::ReadCompleteFile]

Categories

(Core :: JavaScript Engine, defect, P3)

defect

Tracking

()

RESOLVED FIXED
154 Branch
Tracking Status
firefox-esr115 --- wontfix
firefox-esr140 --- wontfix
firefox-esr153 --- wontfix
firefox152 --- wontfix
firefox153 --- wontfix
firefox154 --- fixed

People

(Reporter: bugmon, Assigned: jandem)

References

(Blocks 2 open bugs)

Details

(Keywords: ai-involved, crash, testcase)

Attachments

(3 files, 1 external file)

js::AutoFile::open in js/src/util/CompleteFile.cpp maps the magic filename "-" to the process-global stdin FILE* without any thread-safety provision. js::ReadCompleteFile then deliberately uses getc_unlocked — the libc primitive that explicitly bypasses FILE* locking — on whatever FILE* it receives. When the public API JS::CompileUtf8Path(cx, opts, "-") is invoked concurrently from multiple threads, all threads execute getc_unlocked on the same global stdin object simultaneously.

The race manifests in glibc's lazy stdin buffer allocation: the first call to getc_unlocked on a fresh stdin triggers _IO_file_underflow → _IO_doallocbuf → _IO_file_doallocate → _IO_setb. When multiple threads enter this path concurrently, multiple buffers are allocated, and _IO_setb frees a buffer that another thread just installed (or is still using). The result is heap chunk metadata corruption: glibc's free() aborts with 'corrupted size vs. prev_size' or 'double free or corruption'.

The trigger in this PoC is the DEBUG-only shell function disfile(), which is registered in the unconditional shell_functions array (not in fuzzing_unsafe_functions) and therefore remains available under --fuzzing-safe. However, the defective code is in SpiderMonkey's util/ layer and is exposed via the public JS::CompileUtf8Path API — any embedder or future caller that exposes this API to threads with attacker-controlled paths inherits the same heap corruption. The fix belongs in CompleteFile.cpp: either reject "-" outright, wrap stdin access with flockfile/funlockfile, or fall back to locking getc when fp == stdin.

Build Info

Affected Code

File: js/src/util/CompleteFile.cpp, line 127-129

bool js::AutoFile::open(JSContext* cx, const char* filename) {
  if (!filename || std::strcmp(filename, "-") == 0) {
    fp_ = stdin;   // BUG: returns process-global stdin, no flockfile, no thread guard
  } else {

File: js/src/util/CompleteFile.cpp, line 43-59

  /* Use the fastest available getc. */
  auto fast_getc =
#if defined(HAVE_GETC_UNLOCKED)
      getc_unlocked      // BUG: explicitly thread-unsafe, bypasses FILE* locking
#elif defined(HAVE__GETC_NOLOCK)
      _getc_nolock
#else
      getc
#endif
      ;

  // Read in the whole file. ...
  for (;;) {
    int c = fast_getc(fp);   // CRASH SITE: races on _IO_doallocbuf when fp == stdin

File: js/src/vm/CompilationAndEvaluation.cpp, line 219-229

JSScript* JS::CompileUtf8Path(JSContext* cx,
                              const ReadOnlyCompileOptions& optionsArg,
                              const char* filename) {
  AutoFile file;
  if (!file.open(cx, filename)) {   // "-" → stdin
    return nullptr;
  }

  CompileOptions options(cx, optionsArg);
  options.setFileAndLine(filename, 1);
  return CompileUtf8File(cx, options, file.fp());   // → ReadCompleteFile(cx, stdin, ...)
}

File: js/src/shell/js.cpp, line 4008

    script = JS::CompileUtf8Path(cx, options, filename.get());  // disfile passes user string directly, no ResolvePath

File: js/src/shell/js.cpp, line 10153-10155

    JS_FN_HELP("disfile", DisassFile, 1, 0,   // in shell_functions[], NOT fuzzing_unsafe_functions[]
"disfile('foo.js')",
"  Disassemble script file into bytecodes.\n"),

The combination of (1) AutoFile::open returning a SHARED process-global resource for "-", and (2) ReadCompleteFile using the THREAD-UNSAFE getc_unlocked on that resource, creates a heap-corruption hazard whenever JS::CompileUtf8Path("-") is reachable from multiple threads. The shell's readline() builtin already guards against this exact getc_unlocked-on-stdin hazard with an explicit worker check at js.cpp:3393; CompileUtf8Path lacks any such guard.

Exploit Chain

  1. Attacker provides JS that spawns multiple worker threads via evalInWorker()
  2. All workers synchronize at a barrier (Atomics.wait/notify on SharedArrayBuffer) to maximize concurrency
  3. Upon barrier release, all workers simultaneously call disfile("-")
  4. DisassFile (js.cpp:4008) calls JS::CompileUtf8Path(cx, opts, "-") — no ResolvePath, raw filename passed through
  5. AutoFile::open (CompleteFile.cpp:128) maps "-" to the global stdin FILE* — same pointer in every thread
  6. CompileUtf8File → ReadCompleteFile (CompleteFile.cpp:59) calls getc_unlocked(stdin) without holding any lock
  7. On the first read of a fresh stdin, glibc lazily allocates the I/O buffer: _IO_file_underflow → _IO_doallocbuf → _IO_file_doallocate
  8. Multiple threads enter _IO_file_doallocate concurrently; each malloc()s a buffer and calls _IO_setb to install it
  9. _IO_setb checks for an existing buffer and free()s it before installing the new one — but another thread just installed that buffer and may still hold a pointer to it
  10. Result: free() of an in-use buffer, or interleaved free()/malloc() corrupting adjacent chunk size headers
  11. glibc's heap integrity check detects corrupted chunk metadata and aborts: 'corrupted size vs. prev_size' or 'double free or corruption'

Steps to Reproduce

  1. Build SpiderMonkey with --enable-debug (disfile is gated on defined(DEBUG) || defined(JS_JITSPEW))
  2. Save the testcase JS to a file
  3. Run: ./js --fuzzing-safe testcase.js < /dev/null
  4. Crash rate is ~45% per run with /dev/null stdin, ~60% with closed stdin; rerun if it does not crash on the first attempt
  5. Observe SIGABRT with glibc message 'corrupted size vs. prev_size' or 'double free or corruption'

Security Impact

  • Severity: Low
  • Attacker capability: Heap metadata corruption (corrupted chunk size headers, double-free) leading to process abort. In the JS shell context this is a controlled abort by glibc's hardening; with weaker allocator hardening or precise heap grooming, double-free primitives are typically convertible to arbitrary write. However, the only known trigger path requires the DEBUG-only disfile() shell builtin and shell-only evalInWorker(), neither of which exists in shipping Firefox. No browser code path passes "-" to JS::CompileUtf8Path with attacker-controlled threading. This is a defense-in-depth fix for a thread-unsafe public API rather than a directly exploitable browser bug.
  • Preconditions: DEBUG (or JS_JITSPEW) build of the SpiderMonkey JS shell; disfile() and evalInWorker() shell builtins; multi-core machine for true parallelism. Not reachable from web content in Firefox.

ASAN Report

=== Crash via js_shell_evaluator (--fuzzing-safe) ===
crashed: true
signal: SIGABRT
stderr: corrupted size vs. prev_size

=== Backtrace (LD_PRELOAD SIGABRT handler) ===
double free or corruption (out)
=== ABORT BACKTRACE ===
/lib/x86_64-linux-gnu/libc.so.6(pthread_kill+0x11c)
/lib/x86_64-linux-gnu/libc.so.6(gsignal+0x1e)
/lib/x86_64-linux-gnu/libc.so.6(abort+0xdf)
/lib/x86_64-linux-gnu/libc.so.6(+0x297b6)                                          # __malloc_assert
/lib/x86_64-linux-gnu/libc.so.6(+0xa8ff5)                                          # malloc_printerr
/lib/x86_64-linux-gnu/libc.so.6(+0xab120)                                          # unlink_chunk / corrupted size check
/lib/x86_64-linux-gnu/libc.so.6(+0xab529)                                          # _int_free
/lib/x86_64-linux-gnu/libc.so.6(__libc_free+0x7e)                                  # free()
/lib/x86_64-linux-gnu/libc.so.6(_IO_setb+0x55)                                     # frees OLD buffer (set by another thread!)
/lib/x86_64-linux-gnu/libc.so.6(_IO_file_doallocate+0xae)                          # allocates new buffer
/lib/x86_64-linux-gnu/libc.so.6(_IO_doallocbuf+0x54)                               # lazy stdin buffer allocation - RACE WINDOW
/lib/x86_64-linux-gnu/libc.so.6(_IO_file_underflow+0x244)
/lib/x86_64-linux-gnu/libc.so.6(_IO_default_uflow+0x32)
/firefox/obj-js-debug/dist/bin/js(+0x3af6834)                                      # js::ReadCompleteFile @ CompleteFile.cpp:59 (getc_unlocked)
/firefox/obj-js-debug/dist/bin/js(JS::CompileUtf8File+0x73)
/firefox/obj-js-debug/dist/bin/js(JS::CompileUtf8Path+0x11e)                       # JS::CompileUtf8Path("-")
/firefox/obj-js-debug/dist/bin/js(+0x26de4ea)                                      # DisassFile (disfile)
... interpreter frames ...
/firefox/obj-js-debug/dist/bin/js(JS_ExecuteScript+0x80)
/firefox/obj-js-debug/dist/bin/js(+0x26fbeef)                                      # WorkerMain @ js.cpp:4736
/firefox/obj-js-debug/dist/bin/js(+0x2722baa)                                      # ThreadTrampoline::Start
/lib/x86_64-linux-gnu/libc.so.6(+0x9caa4)                                          # start_thread
/lib/x86_64-linux-gnu/libc.so.6(__clone+0x44)

=== addr2line resolution ===
0x3af6834 → js::ReadCompleteFile(JSContext*, _IO_FILE*, mozilla::Vector<unsigned char, 8ul, js::TempAllocPolicy>&)
            /firefox/js/src/util/CompleteFile.cpp:59
0x26fbeef → WorkerMain(std::unique_ptr<WorkerInput, JS::DeletePolicy<WorkerInput> >)
            /firefox/js/src/shell/js.cpp:4736

=== Reproducibility (multiple runs) ===
stdin = /dev/null:   9/20 crashed (45%)
stdin = closed pipe: 6/10 crashed (60%)
Alternate crash messages observed: 'double free or corruption (!prev)', 'double free or corruption (out)', 'corrupted size vs. prev_size'
Group: core-security → javascript-core-security
Attached file tmpxcvc6i82.js —
Attached patch fix.patch — — Splinter Review
Attached file crash_stack.txt —
Severity: -- → S4
Priority: -- → P3

This is just a JS shell issue.

Group: javascript-core-security
Keywords: sec-low

This is faster, simpler, and avoids the locking issue for stdin.

Assignee: nobody → jdemooij
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Pushed by jdemooij@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/814b01c24441 https://hg.mozilla.org/integration/autoland/rev/c4285c5db24c Use fread instead of unlocked getc in ReadCompleteFile. r=spidermonkey-reviewers,jonco
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → 154 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: