fix: enforce the existing crash dump limit during reading

This commit is contained in:
m4air
2026-09-16 18:52:34 -07:00
parent 75d9b38bdd
commit baed0bb23c
6 changed files with 568 additions and 3 deletions
+80
View File
@@ -0,0 +1,80 @@
# Crashpad file-read limit
The local crash-report reader checked a dump's directory-stat size against its existing
64 MiB limit, then later read the path without a limit. A file that grew or was replaced
between those operations could allocate more than 64 MiB in the main process. The same
stat/filter/read sequence exists in `v1.4.198`.
This patch reuses `readNodeFileWithinLimit` at the read itself. An oversized read skips only
that candidate, releases its reservation, and allows selection of the next valid dump. A
successful result reports the byte count actually parsed. The quota, claim policy, other
I/O error handling and partial-header retry policy remain unchanged.
## Reproduction
```sh
ORCA_BACKGROUND_LAUNCH=1 node docs/audits/crashpad-read-limit/reproduce.mjs
```
The runner reverses only `fix.patch` in memory through a temporary Vite configuration; it
never edits production source. Both source versions must match the expected SHA-256 hashes.
It runs the actual capture function, parser and filesystem against synthetic temporary files
using the committed `crashpad-capture-read-limit.test.ts`. No ignored notes, native crash,
real crash dump, app window, network request or external host is needed. Largest file: **65 MiB**.
| Phase | Pass | Fail | Skip | Exit |
| -------- | ---: | ---: | ---: | ---: |
| Baseline | 4 | 3 | 1 | 1 |
| Fixed | 8 | 0 | 0 | 0 |
Seven cases run against both versions. Baseline failures are growth/replacement beyond the
limit and stale size metadata after allowed growth. The two oversized reads delivered
68,157,440-byte buffers to the real parser despite the 67,108,864-byte limit. The fixed version
skips them and selects the next 131-byte renderer dump.
The eighth case exercises **growth after the same open descriptor's stat**. It is fixed-only:
the old `readFile` call does not expose that descriptor-stat boundary, so the baseline skips
it rather than manufacturing a failure. This case grows a real file to 65 MiB, verifies no
oversized buffer reaches the parser, checks both descriptors close, and confirms that a later
valid same-path file remains available after the failed read releases its reservation.
Other controls accept exactly 64 MiB, skip an already oversized file, and preserve existing
partial-header behavior. A short invalid header is rejected only for the current capture
window; a fresh capture can retry it. Promotion to a different completed path can recover
within the original polling window. The proof does not add a file-completion policy.
`results.json` records source hashes, exact counts, failed case names, process exit codes and
non-timeout status. The runner refuses changed source hashes or unexpected results.
## Reachability and memory scope
Crashpad's macOS database prepares a `.dmp` in its `new` directory while writing, then renames
it after completion. Orca recursively scans these directories. Crashpad's seekable writer
keeps the signature invalid until the body is complete, then rewrites the header. The growth
fixture follows that ordering: a small invalid-header file is statted, grows, then receives
its completed header before capture reads it. These are upstream implementation facts, not an
exact vendored-build or field-incident reproduction. See the primary
[database implementation](https://chromium.googlesource.com/crashpad/crashpad/+/refs/heads/main/client/crash_report_database_mac.mm)
and [minidump writer](https://chromium.googlesource.com/crashpad/crashpad/+/HEAD/minidump/minidump_file_writer.cc).
Same-path replacement is a separate admitted filesystem race; natural UUID reuse is not claimed.
This fixes a potentially large **transient allocation after a process crash**. The parser
returns a bounded text signature and does not retain the whole dump. The bounded reader limits
individual buffer capacity and returned content; old and expanded buffers may briefly coexist,
and independent crash captures can overlap. This is not a claim of a 64 MiB aggregate RSS cap,
a long-lived leak, or an explanation of #19831/#19768's reported memory growth.
## Validation and applicability
```sh
ORCA_BACKGROUND_LAUNCH=1 node node_modules/vitest/vitest.mjs run --config config/vitest.config.ts src/main/crash-reporting/crashpad-capture-read-limit.test.ts src/main/crash-reporting/crashpad-capture.test.ts src/main/crash-reporting/minidump-crash-signature.test.ts src/shared/node-bounded-file-reader.test.ts
ORCA_BACKGROUND_LAUNCH=1 node node_modules/typescript/bin/tsc --noEmit -p config/tsconfig.node.json
```
The focused suites pass **51 tests across four files**, including the existing bounded-reader
failure/descriptor-close controls. Node typecheck, focused lint and formatting pass.
The caller before this patch is byte-identical on main
`291b4ddd6f1c1af480169885e0fda7f9c78ff053`, and the reused bounded reader exists there. The exact
production patch passes an alternate-index apply check against that base; no earlier audit
fix is required.
+43
View File
@@ -0,0 +1,43 @@
diff --git a/src/main/crash-reporting/crashpad-capture.ts b/src/main/crash-reporting/crashpad-capture.ts
index 1f628fe9c2..3f936f0942 100644
--- a/src/main/crash-reporting/crashpad-capture.ts
+++ b/src/main/crash-reporting/crashpad-capture.ts
@@ -7,9 +7,13 @@
// a CHECK failure becomes nameable without shipping raw memory anywhere.
import type { Dirent } from 'node:fs'
-import { readdir, readFile, rm, stat } from 'node:fs/promises'
+import { readdir, rm, stat } from 'node:fs/promises'
import path from 'node:path'
import { app, crashReporter } from 'electron'
+import {
+ NodeFileReadTooLargeError,
+ readNodeFileWithinLimit
+} from '../../shared/node-bounded-file-reader'
import {
parseMinidumpCrashSignature,
type MinidumpCrashSignature
@@ -292,7 +296,8 @@ export async function captureMinidumpSignature(
}
reservedDumpPaths.add(dump.filePath)
try {
- const signature = parseMinidumpCrashSignature(await readFile(dump.filePath), {
+ const { buffer } = await readNodeFileWithinLimit(dump.filePath, MAX_DUMP_BYTES)
+ const signature = parseMinidumpCrashSignature(buffer, {
expectedProcessType: options.expectedProcessType
})
if (
@@ -304,7 +309,12 @@ export async function captureMinidumpSignature(
return null
}
claimedDumpPaths.set(dump.filePath, dump.mtimeMs)
- return { filePath: dump.filePath, sizeBytes: dump.size, signature }
+ return { filePath: dump.filePath, sizeBytes: buffer.byteLength, signature }
+ } catch (error) {
+ if (error instanceof NodeFileReadTooLargeError) {
+ return null
+ }
+ throw error
} finally {
reservedDumpPaths.delete(dump.filePath)
}
@@ -0,0 +1,164 @@
import { createHash } from 'node:crypto'
import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'
import { createRequire } from 'node:module'
import { tmpdir } from 'node:os'
import { join, resolve } from 'node:path'
import { fileURLToPath, pathToFileURL } from 'node:url'
import { applyPatch, parsePatch, reversePatch } from 'diff'
import { build } from 'esbuild'
if (process.env.ORCA_BACKGROUND_LAUNCH !== '1') {
throw new Error('Run with ORCA_BACKGROUND_LAUNCH=1.')
}
const root = fileURLToPath(new URL('../../../', import.meta.url))
const patch = await readFile(new URL('./fix.patch', import.meta.url), 'utf8')
const expectedSourceHashes = {
before: 'f0326e6be534a321adc765bc0bf95ef72debe5ac701861c82e96b87ec822f022',
after: '3d6ec639f4849944dc73f7c9d73c243bc34375ce8e2ba1584d6350e91763edbd'
}
const beforeSources = {}
const sourceHashes = {}
for (const parsed of parsePatch(patch)) {
const path = parsed.newFileName.replace(/^b\//, '')
const absolute = resolve(root, path)
const current = await readFile(absolute, 'utf8')
const before = applyPatch(current, reversePatch(parsed))
if (before === false) {
throw new Error(`Source changed; review the proof patch: ${path}`)
}
beforeSources[absolute.replaceAll('\\', '/')] = before
sourceHashes[path] = {
before: createHash('sha256').update(before).digest('hex'),
after: createHash('sha256').update(current).digest('hex')
}
if (
sourceHashes[path].before !== expectedSourceHashes.before ||
sourceHashes[path].after !== expectedSourceHashes.after
) {
throw new Error(`Source hash changed; review this evidence: ${path}`)
}
}
for (const path of [
'src/main/crash-reporting/crashpad-capture-read-limit.test.ts',
'src/main/crash-reporting/minidump-crash-signature.ts',
'src/shared/node-bounded-file-reader.ts'
]) {
sourceHashes[path] = {
current: createHash('sha256')
.update(await readFile(resolve(root, path)))
.digest('hex')
}
}
const scratch = await mkdtemp(join(tmpdir(), 'orca-crashpad-read-limit-'))
const require = createRequire(import.meta.url)
let runnerModuleId
try {
const runnerPath = join(scratch, 'run-process.cjs')
await build({
absWorkingDir: root,
entryPoints: [resolve(root, 'src/shared/child-process/run-process.ts')],
outfile: runnerPath,
bundle: true,
platform: 'node',
format: 'cjs',
logLevel: 'silent'
})
runnerModuleId = require.resolve(runnerPath)
const { runProcess } = require(runnerModuleId)
const baselineConfig = join(scratch, 'before.config.mjs')
const fixedConfig = join(scratch, 'after.config.mjs')
const includes = ['src/main/crash-reporting/crashpad-capture-read-limit.test.ts']
const configImport = JSON.stringify(pathToFileURL(resolve(root, 'config/vitest.config.ts')).href)
await writeFile(
baselineConfig,
`import base from ${configImport};
const beforeSources = ${JSON.stringify(beforeSources)};
export default {...base, test: {...base.test, include: ${JSON.stringify(includes)}, testNamePattern: /^(?!bounds same-open growth)/}, plugins: [{
name: 'crashpad-read-limit-before-fix', enforce: 'pre',
transform(_code, id) {
const before = beforeSources[id.replaceAll('\\\\', '/').split('?')[0]];
return before === undefined ? null : {code: before, map: null};
}
}]};\n`
)
await writeFile(
fixedConfig,
`import base from ${configImport};\nexport default {...base, test: {...base.test, include: ${JSON.stringify(includes)}}};\n`
)
async function run(label, config) {
const report = join(scratch, `${label}.json`)
const result = await runProcess({
program: process.execPath,
args: [
resolve(root, 'node_modules/vitest/vitest.mjs'),
'run',
'--config',
config,
'--reporter=json',
`--outputFile=${report}`
],
cwd: root,
env: process.env,
timeoutMs: 90_000,
maxOutputBytes: 4 * 1024 * 1024
})
let parsed
try {
parsed = JSON.parse(await readFile(report, 'utf8'))
} catch (error) {
throw new Error(`${label} runner failed: ${result.stderr || result.stdout}`, { cause: error })
}
return {
exitCode: result.code,
timedOut: result.timedOut,
passed: parsed.numPassedTests,
failed: parsed.numFailedTests,
skipped: parsed.numPendingTests,
failedCases: parsed.testResults.flatMap((suite) =>
suite.assertionResults
.filter((test) => test.status === 'failed')
.map((test) => test.fullName)
)
}
}
const before = await run('before', baselineConfig)
const after = await run('after', fixedConfig)
const passed =
before.exitCode === 1 &&
!before.timedOut &&
before.failed === 3 &&
before.passed === 4 &&
before.skipped === 1 &&
after.exitCode === 0 &&
!after.timedOut &&
after.passed === 8 &&
after.failed === 0 &&
after.skipped === 0
const result = {
comparison:
'Actual crashpad capture and parser with temporary synthetic files; before reverses only fix.patch in memory; descriptor-growth control is fixed-only',
sourceHashes,
before,
after,
passed
}
await writeFile(
new URL('./results.json', import.meta.url),
`${JSON.stringify(result, null, 2)}\n`
)
console.log(JSON.stringify(result, null, 2))
if (!passed) {
process.exitCode = 1
}
} finally {
if (runnerModuleId) {
delete require.cache[runnerModuleId]
}
await rm(scratch, { recursive: true, force: true })
}
@@ -0,0 +1,39 @@
{
"comparison": "Actual crashpad capture and parser with temporary synthetic files; before reverses only fix.patch in memory; descriptor-growth control is fixed-only",
"sourceHashes": {
"src/main/crash-reporting/crashpad-capture.ts": {
"before": "f0326e6be534a321adc765bc0bf95ef72debe5ac701861c82e96b87ec822f022",
"after": "3d6ec639f4849944dc73f7c9d73c243bc34375ce8e2ba1584d6350e91763edbd"
},
"src/main/crash-reporting/crashpad-capture-read-limit.test.ts": {
"current": "0d285d75aa09b5ccbdb38d4360e29b6b5f74e43a75c803c2aefeeb190684036d"
},
"src/main/crash-reporting/minidump-crash-signature.ts": {
"current": "91a87850fd5b303cc1607825886f8ee43c68b15e5367d19632726fa93aa48c8d"
},
"src/shared/node-bounded-file-reader.ts": {
"current": "2cffa451b72d059226e0c2d4b6d2a5203cb8ddafb358d5e828dcd3436d197ecb"
}
},
"before": {
"exitCode": 1,
"timedOut": false,
"passed": 4,
"failed": 3,
"skipped": 1,
"failedCases": [
"skips growth past existing limit and captures next valid dump",
"skips replacement past existing limit and captures next valid dump",
"records bytes actually parsed when permitted growth follows directory stat"
]
},
"after": {
"exitCode": 0,
"timedOut": false,
"passed": 8,
"failed": 0,
"skipped": 0,
"failedCases": []
},
"passed": true
}
@@ -0,0 +1,229 @@
import type * as FsModule from 'node:fs/promises'
import type * as ParserModule from './minidump-crash-signature'
import { mkdtemp, mkdir, open, rm, rename, truncate, utimes, writeFile } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { afterEach, beforeEach, expect, it, vi } from 'vitest'
const state = vi.hoisted(() => {
const callbacks: {
afterStat?: (path: string) => Promise<void>
afterOpenStat?: (path: string) => Promise<void>
} = {}
const parsedBytes: number[] = []
const closedPaths: string[] = []
return { callbacks, parsedBytes, closedPaths }
})
vi.mock('electron', () => ({
app: { getPath: () => '/unused' },
crashReporter: { start: vi.fn() }
}))
vi.mock('node:fs/promises', async (original) => {
const fs = await original<typeof FsModule>()
return {
...fs,
open: async (...args: Parameters<typeof fs.open>) => {
const handle = await fs.open(...args)
const readStat = handle.stat.bind(handle)
const close = handle.close.bind(handle)
return Object.assign(handle, {
stat: async () => {
const stats = await readStat()
await state.callbacks.afterOpenStat?.(String(args[0]))
return stats
},
close: async () => {
await close()
state.closedPaths.push(String(args[0]))
}
})
},
stat: async (...args: Parameters<typeof fs.stat>) => {
const stats = await fs.stat(...args)
await state.callbacks.afterStat?.(String(args[0]))
return stats
}
}
})
vi.mock('./minidump-crash-signature', async (original) => {
const parser = await original<typeof ParserModule>()
return {
...parser,
parseMinidumpCrashSignature: (
...args: Parameters<typeof parser.parseMinidumpCrashSignature>
) => {
state.parsedBytes.push(args[0].byteLength)
return parser.parseMinidumpCrashSignature(...args)
}
}
})
import { _setCrashpadCaptureStateForTest, captureMinidumpSignature } from './crashpad-capture'
const LIMIT = 64 * 1024 * 1024
const CRASHED_AT = 1_700_000_000_000
let directory: string
function rendererDump() {
const dump = Buffer.alloc(131)
dump.writeUInt32LE(0x504d444d, 0)
dump.writeUInt32LE(0xa793, 4)
dump.writeUInt32LE(1, 8)
dump.writeUInt32LE(32, 12)
dump.writeUInt32LE(0x43500001, 32)
dump.writeUInt32LE(52, 36)
dump.writeUInt32LE(44, 40)
dump.writeUInt32LE(1, 44)
dump.writeUInt32LE(12, 80)
dump.writeUInt32LE(96, 84)
dump.writeUInt32LE(1, 96)
dump.writeUInt32LE(108, 100)
dump.writeUInt32LE(118, 104)
dump.writeUInt32LE(5, 108)
dump.write('ptype', 112)
dump.writeUInt32LE(8, 118)
dump.write('renderer', 122)
return dump
}
async function file(name: string, newer = false, bytes = rendererDump()) {
const path = join(directory, 'new', name)
await writeFile(path, bytes)
await utimes(path, CRASHED_AT / 1000, (CRASHED_AT + (newer ? 100 : 0)) / 1000)
return path
}
function capture() {
return captureMinidumpSignature(CRASHED_AT, {
expectedProcessType: 'renderer',
timeoutMs: 0,
now: () => CRASHED_AT
})
}
beforeEach(async () => {
directory = await mkdtemp(join(tmpdir(), 'orca-crashpad-limit-'))
await mkdir(join(directory, 'new'))
state.callbacks.afterStat = undefined
state.callbacks.afterOpenStat = undefined
state.parsedBytes.length = 0
state.closedPaths.length = 0
_setCrashpadCaptureStateForTest({ dumpDirectory: directory, started: true })
})
afterEach(async () => {
_setCrashpadCaptureStateForTest(null)
await rm(directory, { recursive: true, force: true })
})
it.each(['growth', 'replacement'] as const)(
'skips %s past existing limit and captures next valid dump',
async (kind) => {
const next = await file('next.dmp')
const unfinished = rendererDump()
unfinished.writeUInt32LE(0, 0)
const race = await file('racing.dmp', true, unfinished)
state.callbacks.afterStat = async (path) => {
if (path !== race) {
return
}
state.callbacks.afterStat = undefined
if (kind === 'replacement') {
await rename(race, join(directory, 'retired.bin'))
await writeFile(race, unfinished)
}
await truncate(race, LIMIT + 1024 * 1024)
const handle = await open(race, 'r+')
try {
await handle.write(rendererDump().subarray(0, 4), 0, 4, 0)
} finally {
await handle.close()
}
}
const result = await capture()
expect(Math.max(0, ...state.parsedBytes)).toBeLessThanOrEqual(LIMIT)
expect(result?.filePath).toBe(next)
}
)
it('accepts exactly the existing 64 MiB limit', async () => {
const path = await file('limit.dmp')
await truncate(path, LIMIT)
const result = await capture()
expect(result?.filePath).toBe(path)
expect(result?.sizeBytes).toBe(LIMIT)
expect(state.parsedBytes).toEqual([LIMIT])
})
it('ignores an already oversize candidate without parsing it', async () => {
const next = await file('next.dmp')
const path = await file('oversize.dmp', true)
await truncate(path, LIMIT + 1024 * 1024)
const result = await capture()
expect(result?.filePath).toBe(next)
expect(state.parsedBytes).toEqual([131])
})
it('documents same-path partial-header rejection for this capture window', async () => {
const path = await file('partial.dmp', false, Buffer.from('MDMP'))
let clock = CRASHED_AT
const result = await captureMinidumpSignature(CRASHED_AT, {
expectedProcessType: 'renderer',
timeoutMs: 500,
now: () => clock,
sleep: async (ms) => {
clock += ms
await writeFile(path, rendererDump())
}
})
expect(result).toBeNull()
expect(state.parsedBytes).toEqual([4])
expect((await capture())?.filePath).toBe(path)
})
it('can recover partial-header rejection after completed-path promotion', async () => {
const path = await file('partial.dmp', false, Buffer.from('MDMP'))
const promoted = join(directory, 'pending', 'partial.dmp')
let clock = CRASHED_AT
const result = await captureMinidumpSignature(CRASHED_AT, {
expectedProcessType: 'renderer',
timeoutMs: 500,
now: () => clock,
sleep: async (ms) => {
clock += ms
await writeFile(path, rendererDump())
await mkdir(join(directory, 'pending'))
await rename(path, promoted)
}
})
expect(result?.filePath).toBe(promoted)
expect(state.parsedBytes).toEqual([4, 131])
})
it('bounds same-open growth after descriptor stat and releases the reservation', async () => {
const next = await file('next.dmp')
const race = await file('racing.dmp', true)
let grew = false
state.callbacks.afterOpenStat = async (path) => {
if (path !== race) {
return
}
state.callbacks.afterOpenStat = undefined
grew = true
await truncate(race, LIMIT + 1024 * 1024)
}
const result = await capture()
expect(grew).toBe(true)
expect(result?.filePath).toBe(next)
expect(state.parsedBytes).toEqual([131])
expect(state.closedPaths).toEqual(expect.arrayContaining([race, next]))
await writeFile(race, rendererDump())
expect((await capture())?.filePath).toBe(race)
})
it('records bytes actually parsed when permitted growth follows directory stat', async () => {
const race = await file('racing.dmp')
state.callbacks.afterStat = async (path) => {
if (path !== race) {
return
}
state.callbacks.afterStat = undefined
await truncate(race, 132)
}
expect((await capture())?.sizeBytes).toBe(132)
expect(state.parsedBytes).toEqual([132])
})
+13 -3
View File
@@ -7,9 +7,13 @@
// a CHECK failure becomes nameable without shipping raw memory anywhere.
import type { Dirent } from 'node:fs'
import { readdir, readFile, rm, stat } from 'node:fs/promises'
import { readdir, rm, stat } from 'node:fs/promises'
import path from 'node:path'
import { app, crashReporter } from 'electron'
import {
NodeFileReadTooLargeError,
readNodeFileWithinLimit
} from '../../shared/node-bounded-file-reader'
import {
parseMinidumpCrashSignature,
type MinidumpCrashSignature
@@ -292,7 +296,8 @@ export async function captureMinidumpSignature(
}
reservedDumpPaths.add(dump.filePath)
try {
const signature = parseMinidumpCrashSignature(await readFile(dump.filePath), {
const { buffer } = await readNodeFileWithinLimit(dump.filePath, MAX_DUMP_BYTES)
const signature = parseMinidumpCrashSignature(buffer, {
expectedProcessType: options.expectedProcessType
})
if (
@@ -304,7 +309,12 @@ export async function captureMinidumpSignature(
return null
}
claimedDumpPaths.set(dump.filePath, dump.mtimeMs)
return { filePath: dump.filePath, sizeBytes: dump.size, signature }
return { filePath: dump.filePath, sizeBytes: buffer.byteLength, signature }
} catch (error) {
if (error instanceof NodeFileReadTooLargeError) {
return null
}
throw error
} finally {
reservedDumpPaths.delete(dump.filePath)
}