fs: fix out-of-bounds write in mkdtemp for long prefixes - #64770

frandle331-yh · 2026-07-27T01:36:52Z

Mkdtemp() allocated the template buffer as length + strlen("XXXXXX"),
leaving no room for the terminating NUL byte that snprintf() writes. For
a single-byte prefix long enough to force the heap allocation path
(length + 6 > the stack-buffer threshold), snprintf() wrote the six 'X'
characters plus its NUL one byte past the end of the buffer -- a 1-byte
heap-buffer-overflow flagged by AddressSanitizer.

Allocate one extra byte for the NUL terminator, matching the + 1
already used by the sibling allocations in the same file.

Signed-off-by: frandle331-yh s1240100@gmail.com

codecov · 2026-07-27T07:56:20Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.13%. Comparing base (54a5095) to head (a702884).
⚠️ Report is 402 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #64770      +/-   ##
==========================================
- Coverage   90.14%   90.13%   -0.01%     
==========================================
  Files         744      744              
  Lines      242518   242519       +1     
  Branches    45685    45685              
==========================================
- Hits       218611   218599      -12     
- Misses      15396    15406      +10     
- Partials     8511     8514       +3

Files with missing lines	Coverage Δ
src/node_file.cc	`74.20% <100.00%> (+<0.01%)`	⬆️

... and 32 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.

Mkdtemp() allocated the template buffer as `length + strlen("XXXXXX")`, leaving no room for the terminating NUL byte. For a single-byte prefix long enough to force the heap allocation path (length + 6 > the stack-buffer threshold), the terminating NUL was written one byte past the end of the buffer -- a 1-byte heap-buffer-overflow flagged by AddressSanitizer. Allocate room for the terminating NUL, copy the suffix, and use SetLengthAndZeroTerminate to set the correct length and write the terminator, following the MaybeStackBuffer paradigm used elsewhere in this file. Signed-off-by: frandle331-yh <s1240100@gmail.com>

Renegade334

Thanks!

frandle331-yh · 2026-07-28T17:57:30Z

It looks like the macOS CI failed due to a timeout in parallel/test-debugger-break, which seems unrelated to my fs.mkdtemp changes (likely a flaky test).
Could you please re-run the CI for me?

nodejs-github-bot · 2026-07-28T18:06:15Z

CI: https://ci.nodejs.org/job/node-test-pull-request/75261/

frandle331-yh · 2026-07-28T20:25:48Z

It looks like the CI failed again (node-test-commit, etc.), but I don't have permission to view the Jenkins logs.
Could anyone please share the error logs with me, or let me know if these failures are related to my changes?

nodejs-github-bot · 2026-08-09T07:54:11Z

CI: https://ci.nodejs.org/job/node-test-pull-request/75683/

nodejs-github-bot · 2026-08-20T23:56:30Z

Landed in 9472421

Mkdtemp() allocated the template buffer as `length + strlen("XXXXXX")`, leaving no room for the terminating NUL byte. For a single-byte prefix long enough to force the heap allocation path (length + 6 > the stack-buffer threshold), the terminating NUL was written one byte past the end of the buffer -- a 1-byte heap-buffer-overflow flagged by AddressSanitizer. Allocate room for the terminating NUL, copy the suffix, and use SetLengthAndZeroTerminate to set the correct length and write the terminator, following the MaybeStackBuffer paradigm used elsewhere in this file. Signed-off-by: frandle331-yh <s1240100@gmail.com> PR-URL: #64770 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>

…ESM move dropped Dependabot #103 proposes node 24.20.0 -> 26.8-alpine. Measured against nodejs.org/dist/index.json: the 24.x line is the CURRENT LTS (Krypton, v24.21.0 released 2026-09-07) and every 26.x release carries `lts: false` — 26 is Current, not LTS. Moving production off a supported line is not a dependency bump, so this takes the security patch WITHIN the LTS line instead: 24.21.0 everywhere. There is a concrete reason to take it. v24.21.0 fixes an out-of-bounds write in `mkdtemp` for long prefixes (nodejs/node#64770), and this repository calls `fs.mkdtempSync` in three places, one of them the evidence path itself (mutation-harness.js gitSupplement). Five pins move together, because regenerate-derived-artefacts.yml REFUSES a mismatch between ci.yml and .github/workflows/.node-version-for-derived: apps/api/Dockerfile (x2), ci.yml, that pin file, and the root engines field. THE DEFENCE THAT WAS LOST. Two defects, both introduced by the ESM move in #113 and both invisible until a full mutation run was attempted: 1. The harness spawns jest directly and did not pass --experimental-vm-modules, so under ESM every battery failed with `SyntaxError: Cannot use import statement outside a module` and the run stopped at `baseline: FAIL`. Measured: 20 suites / 394 tests pass with the flag, 20 suites fail with 0 tests without it. `pnpm test` sets the flag; the harness never did. It went unnoticed because the only caller of a full run is regenerate-derived-artefacts.yml, gated on author == 'dependabot[bot]' — it never ran for #113. NODE_OPTIONS is appended, not assigned, so an inherited flag survives. 2. `isolatedModules: true` on the ts-jest transform removes type-checking at transform time, and nine of the 85 mutants were KILLED_BY_TYPECHECK. Measured with the flag: 79 killed by test, 6 SURVIVED — #11, #15, #16, #50, #52, #75, each a mutant the compiler used to reject and the tests do not catch. Three of the nine (#6, #13, #21) turned out to be caught by tests after all; six were not. A mutation baseline that records 0 survivors while the tree actually has 6 is worse than no baseline. The flag is removed. Its cost, measured: the local suite goes from 26s to 58s, and the CI job from about 3 minutes to an expected 3.5-4. Six mutants of the canonical-JSON and validator paths are worth thirty seconds. Measured after, from the repository root (the invocation the evidence records): 85 sites, 76 killed by test, 9 killed by compiler, 0 survived — identical to the counts on main. `--verify-structure` prints "Structurally verified 85 source-bound mutation records" with no TOOLCHAIN_MISMATCH, because tools now read node v24.21.0. tsc --noEmit 0 under both tsconfig.json and tsconfig.test.json. Closes #103 by answering it: the upgrade it asks for is refused with a measurement and the security patch it was reaching for is taken.

…ESM move dropped (#114) * fix: stay on the Node LTS line, and restore the mutation defence the ESM move dropped Dependabot #103 proposes node 24.20.0 -> 26.8-alpine. Measured against nodejs.org/dist/index.json: the 24.x line is the CURRENT LTS (Krypton, v24.21.0 released 2026-09-07) and every 26.x release carries `lts: false` — 26 is Current, not LTS. Moving production off a supported line is not a dependency bump, so this takes the security patch WITHIN the LTS line instead: 24.21.0 everywhere. There is a concrete reason to take it. v24.21.0 fixes an out-of-bounds write in `mkdtemp` for long prefixes (nodejs/node#64770), and this repository calls `fs.mkdtempSync` in three places, one of them the evidence path itself (mutation-harness.js gitSupplement). Five pins move together, because regenerate-derived-artefacts.yml REFUSES a mismatch between ci.yml and .github/workflows/.node-version-for-derived: apps/api/Dockerfile (x2), ci.yml, that pin file, and the root engines field. THE DEFENCE THAT WAS LOST. Two defects, both introduced by the ESM move in #113 and both invisible until a full mutation run was attempted: 1. The harness spawns jest directly and did not pass --experimental-vm-modules, so under ESM every battery failed with `SyntaxError: Cannot use import statement outside a module` and the run stopped at `baseline: FAIL`. Measured: 20 suites / 394 tests pass with the flag, 20 suites fail with 0 tests without it. `pnpm test` sets the flag; the harness never did. It went unnoticed because the only caller of a full run is regenerate-derived-artefacts.yml, gated on author == 'dependabot[bot]' — it never ran for #113. NODE_OPTIONS is appended, not assigned, so an inherited flag survives. 2. `isolatedModules: true` on the ts-jest transform removes type-checking at transform time, and nine of the 85 mutants were KILLED_BY_TYPECHECK. Measured with the flag: 79 killed by test, 6 SURVIVED — #11, #15, #16, #50, #52, #75, each a mutant the compiler used to reject and the tests do not catch. Three of the nine (#6, #13, #21) turned out to be caught by tests after all; six were not. A mutation baseline that records 0 survivors while the tree actually has 6 is worse than no baseline. The flag is removed. Its cost, measured: the local suite goes from 26s to 58s, and the CI job from about 3 minutes to an expected 3.5-4. Six mutants of the canonical-JSON and validator paths are worth thirty seconds. Measured after, from the repository root (the invocation the evidence records): 85 sites, 76 killed by test, 9 killed by compiler, 0 survived — identical to the counts on main. `--verify-structure` prints "Structurally verified 85 source-bound mutation records" with no TOOLCHAIN_MISMATCH, because tools now read node v24.21.0. tsc --noEmit 0 under both tsconfig.json and tsconfig.test.json. Closes #103 by answering it: the upgrade it asks for is refused with a measurement and the security patch it was reaching for is taken. * receipt(#103): file the ChangeAdmissionReceipt/v1 for the Node LTS pin Draft by the bundle's own verify.py in diff mode: verdict admitted, 3 entities, all three verified, zero exemptions — nothing needed exempting, which is the first receipt in this series that can say so. One finding came out of it and was fixed rather than exempted. The first draft reported v-config-schema UNDECLARED_CONFIG_KEY NODE_OPTIONS as failed, because the harness fix read process.env.NODE_OPTIONS to append to an inherited value. The verifier was right: reading it makes it a configuration key this file consumes, and declaring a test-harness runtime flag in .env.example — the application's own configuration — would have been the worse answer. The flag is now set outright, and regenerate-derived-artefacts.yml (the only caller of a full run) sets no NODE_OPTIONS, so nothing is lost. Filed in the tree with the receipt included in its own change_set, and the evidence's trackedTreeWithoutEvidence rebound after staging it — the MUN-0049 precedent. schema_check.py: CONFORMANT.

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to file-system APIs and the fs module. needs-ci PRs that need a full CI run. labels Jul 27, 2026

Renegade334 requested changes Jul 27, 2026

View reviewed changes

Comment thread src/node_file.cc Outdated

frandle331-yh force-pushed the fix-mkdtemp-oob-write branch from 7bcec4a to a702884 Compare July 27, 2026 20:24

Renegade334 approved these changes Jul 27, 2026

View reviewed changes

Renegade334 added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Jul 27, 2026

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Jul 27, 2026

This comment has been minimized.

Sign in to view

trivikr added the commit-queue PRs queued for automated landing through the Commit Queue. label Aug 20, 2026

nodejs-github-bot merged commit 9472421 into nodejs:main Aug 20, 2026
74 of 77 checks passed

nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Aug 20, 2026

github-actions Bot mentioned this pull request Aug 25, 2026

2026-08-26, Version 26.8.0 (Current) #65551

Merged

github-actions Bot mentioned this pull request Sep 7, 2026

2026-09-08, Version 24.21.0 'Krypton' (LTS) #65873

Merged

Arcanada mentioned this pull request Sep 14, 2026

fix: stay on the Node LTS line, and restore the mutation defence the ESM move dropped Arcanada-one/muneral#114

Merged

Navigation Menu

Uh oh!

Uh oh!

fs: fix out-of-bounds write in mkdtemp for long prefixes - #64770