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