DepsLog::Recompact() heap-OOB read on corrupted .ninja_deps with bogus out_id (Windows: 0xC0000005 + minidump cascade)

I added support for ninja in zccache and now we are hitting this bug.

TL;DR

ninja -t recompact crashes with a heap-out-of-bounds read in DepsLog::Recompact() when Load() successfully "recovers" from a corrupted .ninja_deps that contains a deps record with an out_id larger than the number of path records that have been ingested so far. The crash is reproducible and deterministic on Windows; downstream tools see a flood of minidump files and a final STATUS_STACK_OVERFLOW (0xC00000FD) that masks the original failure.

This was triaged on ninja 1.13.0 as shipped by the ninja PyPI wheel (which packages Kitware/ninja kitware-staged-features — diff against ninja-build/ninja:master is just the meson jobserver-pipe patch, so this analysis applies equally to upstream master).

Root cause: missing out_id bounds check in DepsLog::Load() propagates into Recompact()

Load() reads each deps record from the file and stores it via UpdateDeps(out_id, deps). The dependency node IDs in the record are bounds-checked against nodes_.size() before use:

https://github.com/ninja-build/ninja/blob/master/src/deps_log.cc#L220-L228

for (int i = 0; i < deps_count; ++i) {
  int node_id = deps_data[i];
  if (node_id >= (int)nodes_.size() || !nodes_[node_id]) {
    read_failed = true;
    break;
  }
}

…but the target out_id (deps_data[0]) is not bounds-checked before UpdateDeps(out_id, deps) is called. UpdateDeps() then unconditionally grows deps_ to out_id + 1 slots:

https://github.com/ninja-build/ninja/blob/master/src/deps_log.cc#L382-L390

bool DepsLog::UpdateDeps(int out_id, Deps* deps) {
  if (out_id >= (int)deps_.size())
    deps_.resize(out_id + 1);
  …
  deps_[out_id] = deps;
  …
}

A corrupted record where the first 4 bytes of the body decode to a bogus out_id (e.g. junk left over from a half-written record) is enough to leave deps_.size() > nodes_.size(). The "premature end of file; recovering" path takes the parser out cleanly, and Load() returns success — at which point the in-memory invariant deps_.size() <= nodes_.size() is broken.

Recompact() then iterates over deps_ and dereferences nodes_[old_id] without checking that index is in range:

https://github.com/ninja-build/ninja/blob/master/src/deps_log.cc#L346-L353

for (int old_id = 0; old_id < (int)deps_.size(); ++old_id) {
  Deps* deps = deps_[old_id];
  if (!deps) continue;

  if (!IsDepsEntryLiveFor(nodes_[old_id]))   // <-- nodes_[old_id] is OOB
    continue;
  …
}

When old_id >= nodes_.size(), nodes_[old_id] is undefined behaviour — on Windows we observe EXCEPTION_ACCESS_VIOLATION (0xC0000005) inside IsDepsEntryLiveFor() (which dereferences the bogus Node*). The same OOB also exists earlier on the !deps branch through nodes_[old_id]->set_id(-1) in the "Clear all known ids" loop a few lines up, but in practice the live-check path is what crashes for us.

Why "valid header + at least one valid record + partial second record" is the necessary trigger

The downstream reporter (FastLED CI on Windows) confirmed empirically that no other corruption shape reproduces:

That matches the analysis above: you need to make it past the header check, ingest enough of the file that Load() returns the "recovered" success path with a non-trivial deps_/nodes_ state, and have at least one bytes-as-int interpretation of the trailing partial record produce an out-of-range out_id. Anything that fails the signature check is dropped via platformAwareUnlink(path.c_str()) before Recompact() runs.

Why it gets into that state in the first place

Any abnormal termination of a previous ninja run mid-OutputForCommand -> RecordDeps -> write leaves the file with exactly that shape. On Windows, common triggers:

• User Ctrl-C during a build.
• Parent process (meson, IDE, CI harness, test runner) kills the ninja process group.
• Two concurrent ninjas with the same build dir (already noted in #1722).
• Compiler/linker OOM-kills the build process which kills ninja.

Not implicated: ccache/zccache — they wrap the compiler invocation and write the depfile (.d) that ninja then parses into .ninja_deps. They never touch .ninja_deps themselves. Verified in the FastLED downstream session by inspecting the zccache-session.log and the deps-log writer path.

Secondary issue: minidump cascade + STATUS_STACK_OVERFLOW on Windows

When the AV fires inside Recompact(), the SEH wrapper in src/ninja.cc calls ExceptionFilter() -> CreateWin32MiniDump(ep):

https://github.com/ninja-build/ninja/blob/master/src/ninja.cc#L1696-L1707 https://github.com/ninja-build/ninja/blob/master/src/ninja.cc#L1934

In practice, a single ninja -t recompact invocation emits ~150 ninja_crash_dump_<pid>.dmp files at ~70 ms intervals (totalling tens of MB), each with the ninja: warning: minidump created: … line interleaved with another ninja: error: exception: 0xC0000005, and terminates after ~12 s with a final ninja: error: exception: 0xC00000FD (STATUS_STACK_OVERFLOW).

Observed sequence (anonymised from the FastLED CI):

3.63 [parent] running ninja -t recompact …
3.67 ninja: warning: premature end of file; recovering
3.67 ninja: error: exception: 0xC0000005
3.74 ninja: warning: minidump created: ninja: error: exception: 0xC0000005
3.82 ninja: warning: minidump created: ninja: error: exception: 0xC0000005
…  (~150 identical lines, ~70 ms apart)
15.89 ninja: error: exception: 0xC00000FD     <-- stack overflow

The interleaved minidump created: … exception: pattern, the ~70 ms spacing, and the eventual STATUS_STACK_OVERFLOW are consistent with MiniDumpWriteDump (via dbghelp.dll) touching the same heap-OOB memory while walking the crashing thread's stack, re-entering the AV path, and being re-caught by SEH — each re-entry pushing a new frame until the thread exhausts its stack. I haven't pinned down where the re-entry happens (likely a dbghelp callback that re-enters the failing dereference while building the memory list for the dump), but the visible effect — one corruption triggers 150 minidumps before the process finally dies — is reproducible.

Suggested mitigations, in priority order:

1. Fix the OOB: bounds-check out_id in Load() before UpdateDeps(out_id, deps); alternatively, in Recompact() guard both nodes_[old_id] accesses with old_id < (int)nodes_.size() && nodes_[old_id]. The latter is the smaller, safer fix and doesn't change the on-disk format semantics.
2. Bound the SEH/minidump cascade: gate CreateWin32MiniDump() behind a std::atomic_flag so the second call no-ops. The current code happily writes another dump from the unwind of a dump-write crash.
3. Optional: have Recompact() skip-and-warn rather than dereference, so a Load()-recovered file at least round-trips through recompact without crashing even if a downstream invariant was already broken.

Reproduction (downstream)

The FastLED project hits this every bash test --cpp run on Windows when their dep-log maintenance step (ninja -t recompact on .build/meson-quick/.ninja_deps) sees a .ninja_deps left over from a previously-killed build. Workaround landed in FastLED PR #2860: quarantine .ninja_deps to .bak before invoking recompact, and touch the maintenance marker on failure so the cascade isn't re-triggered on every invocation. That's a workaround, not a fix — happy to test a patch against upstream.

Related history:

• #1722 (open) — original "segfault reading DepsLog" report; the in-Load() OOB was fixed in 7f6ae667 ("Make the deps_log parser report corrupted files instead of crashing"), but the out_id -> Recompact() path described above survived that fix.
• #595, #805, #2117 — older deps-log corruption reports.
• #262 — original minidump-on-Windows infrastructure (the SEH/minidump bug above is in code that was added here).