fix: eliminate TOCTOU Race Condition (Hackerone report #3407207)

[giulioAZ]

Summary

Fixes a Time-of-Check Time-of-Use race condition in worker thread process.cwd() caching where the counter is incremented before the directory change completes, allowing workers to cache stale directory values.

Description: The main thread's process.chdir() wrapper previously incremented the shared counter before calling the actual chdir(), creating a race window where workers could read the old directory but cache it with the new counter value. This caused subsequent cwd() calls to return incorrect paths until the next chdir().

Proof of Concept: included in the test/parallel/test-worker-cwd-race-condition.mts and already reviewed and validated by Node.js security team here (https://hackerone.com/reports/3407207).

Fix: this fix reorders the operations to change the directory first, then increment the counter, ensuring workers are only notified after the directory change completes.

CVSS 3.0: 3.6 (Low) - AV:L/AC:L/PR:L/UI:N/S:U/C:L/I:L/A:N

---

Problem

In lib/internal/worker.js, the main thread's process.chdir() wrapper increments the shared counter before calling the actual chdir():

process.chdir = function(path) {
  AtomicsAdd(cwdCounter, 0, 1);  // Signal workers first
  originalChdir(path);            // Change directory second
};

This creates a race window where:

Counter increments (workers see "cwd changed")
Worker calls cwd() during this window
Worker reads old directory but caches it with new counter
Directory actually changes
Worker's cache is now stale and persists until next chdir()

Race Window:

┌───────
│ Step   │ [Main Thread]          │ [Worker Thread]              │
├────────
│ Step 1 │ AtomicsAdd(counter, 1) │ → counter = 5                │
│ Step 2 │                        │ AtomicsLoad(counter) → 5     │
│ Step 3 │                        │ cachedCwd = cwd() → /old     │
│ Step 4 │ originalChdir("/new")  │ → CWD = /new                 │
├──────
│ Result │ CWD is /new            │ Cached /old with counter=5  (Wrong)│
└───────
Once cached, subsequent cwd() calls return the stale value:
if (currentCounter === lastCounter) // 5 === 5 ✓
  return cachedCwd; // Returns "/old" when CWD is actually "/new"

Solution

Swap the order - change directory first, then increment counter:

process.chdir = function(path) {
  originalChdir(path);            // Change directory first
  AtomicsAdd(cwdCounter, 0, 1);  // Signal workers second
};

This ensures workers are only notified after the directory change completes, transforming the race into safe eventual consistency.

After Fix:

┌───────
│ Step   │ [Main Thread]          │ [Worker Thread]            │
├────────────
│ Step 1 │ originalChdir("/new")  │ → CWD = /new               │
│ Step 2 │                        │ AtomicsLoad(counter) → 5   │
│ Step 3 │                        │ cachedCwd = cwd() → /new   │
│ Step 4 │ AtomicsAdd(counter, 1) │ → counter = 6              │
├─────────
│ Result │ CWD is /new            │ Cached /new (Correct)    │

Impact

• Affected applications: Multi-threaded Node.js applications using process.chdir() with worker threads, including:
  • Build systems processing multiple projects in parallel
  • CI/CD platforms executing concurrent build jobs
  • Multi-tenant services using directory-based isolation
  • Applications without container or filesystem namespace separation
• Confidentiality: Workers may read files from unintended directories
• Integrity: File operations (read/write) may target wrong directory context
• Severity: Low (CVSS 3.6) - requires specific usage patterns by the application and concurrent execution
• Exploitability: Race window is 1-10 microseconds but consistently reproducible in testing

Proof of Concept

Tested with test/parallel/test-worker-cwd-race-condition.mts on Node.js v25.0.0:

Before fix:
Test case: real process
cwd in worker thread reflected stale value
errors: 54.28% (311/573 races)
After fix:
Test case: fixed process
cwd in worker thread always had expected value
errors: 0% (0/832 races)

Performance Impact

Negligible: same atomic operations, just reordered:
Cache hit rate: Unchanged
Latency: Workers may cache fresh data slightly longer (safer)
Thread safety: Maintained via atomic operations

Related

Original implementation: commit 1d022e8 (April 14, 2019, PR #27224)
CWE-367: Time-of-Check Time-of-Use (TOCTOU) Race Condition
CVSS 3.0: 3.6 (Low) - AV:L/AC:L/PR:L/UI:N/S:U/C:L/I:L/A:N
HackerOne Report: https://hackerone.com/reports/3407207

Credits

Giulio Comi, Caleb Everett, Utku Yildirim

[Renegade334]

This doesn't guarantee execution order either; that would require wait/notify.

[giulioAZ]

execution order is guaranteed (originalChdir is a blocking syscall that completes before AtomicsAdd executes). the Atomic provides a memory barrier for cross-thread.
the current fix (lines swapped) already ensures the cwdCounter only signals freshness after the directory actually changes. This s turning the race condition (workers cache stale data as current) into a safe eventual consistency (workers may be conservative but never trust stale data).
the test results confirm 0% error rate across the 832 races of the PoC, what race condition do you see remaining?

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.74%. Comparing base (7bd2fea) to head (98018fc).

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #61664      +/-   ##
==========================================
- Coverage   89.74%   89.74%   -0.01%     
==========================================
  Files         674      674              
  Lines      204360   204360              
  Branches    39270    39263       -7     
==========================================
- Hits       183409   183405       -4     
- Misses      13260    13263       +3     
- Partials     7691     7692       +1     

Files with missing lines	Coverage Δ
lib/internal/worker.js	96.52% <100.00%> (ø)

... and 31 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.