From 09965f93aeb519a7a699474c83721ce2bd49986f Mon Sep 17 00:00:00 2001 From: iamtoruk Date: Mon, 17 Aug 2026 17:28:41 -0700 Subject: [PATCH] fix(dsh): cap zstd decode, coerce usage fields, scope the snap read Security audit follow-ups on the DeepSeek Harness provider. - **Decompression bomb.** Every zstd frame was decoded with no output bound, so a 16 KB crafted log expanded to ~916 MB of RSS (a 65 KB one declares 2 GB). Each frame now decodes under a 64 MB per-call cap, and the caps chain into a running per-file budget of MAX_SESSION_FILE_BYTES: a frame is given only the bytes the file has left, so node throws ERR_BUFFER_TOO_LARGE without allocating past the cap. The throw propagates out of the existing skip path, which discards the whole file rather than counting the frames read before the bomb, so a crafted tail cannot poison a partial total. The discovery header read takes the same per-frame cap. Measured on a 65 KB / 2 GB bomb: 916 MB -> 67 MB peak, zero calls emitted, one notice. Lines are still materialized eagerly; the byte budget bounds that, and making the read lazy would change readEventLines' contract for no further bound. - **Usage type confusion.** Token fields were read with `?? 0` and never type-checked, so a string or array inputTokens flowed into the global totals and the persisted cache, where `0 + [1, 2]` becomes "01,2". They now go through numberOrZero (copilot.ts semantics: finite, positive, else 0). All-zero calls are still skipped. - **Snap over-scope.** The personal-files read entry is `$HOME/.dsh/sessions` rather than all of `$HOME/.dsh`; the provider reads nothing else. - **Third-party notice.** scanZstdFrames is transcribed from @deepseek-ai/dsh-session-persistence-jsonl. The published npm package is BSD-3-Clause (Copyright (c) 2026, DeepSeek) while the monorepo source declares MIT for the same package; THIRD_PARTY_NOTICES.md reproduces the stricter of the two and ships via package.json `files`. --- THIRD_PARTY_NOTICES.md | 51 +++++++++++++++++++++++++ app/package.json | 2 +- package.json | 1 + src/providers/dsh.ts | 53 ++++++++++++++++++++------ tests/providers/dsh.test.ts | 76 ++++++++++++++++++++++++++++++++++++- 5 files changed, 168 insertions(+), 15 deletions(-) create mode 100644 THIRD_PARTY_NOTICES.md diff --git a/THIRD_PARTY_NOTICES.md b/THIRD_PARTY_NOTICES.md new file mode 100644 index 00000000..c1ffcb8e --- /dev/null +++ b/THIRD_PARTY_NOTICES.md @@ -0,0 +1,51 @@ +# Third-party notices + +CodeBurn is MIT licensed (see `LICENSE`). It also contains code derived from the +projects below, which carry their own terms. Each notice is reproduced here as +those terms require. + +--- + +## @deepseek-ai/dsh-session-persistence-jsonl + +`scanZstdFrames` in `src/providers/dsh.ts` is a transcription of the function of +the same name in this package (`src/zstd.ts`), which is what lets CodeBurn read +a DeepSeek Harness session log without depending on the harness itself. No other +part of the package is used. + +Upstream declares two different licenses for this package: the published npm +package (0.0.1-rc.1) ships a BSD 3-Clause `LICENSE` and declares +`"license": "BSD-3-Clause"`, while the monorepo source it is built from +(`deepseek-ai/deepseek-harness`, `packages/session/session-persistence-jsonl`) +declares MIT. The stricter of the two is reproduced below. + +``` +BSD 3-Clause License + +Copyright (c) 2026, DeepSeek + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are met: + +1. Redistributions of source code must retain the above copyright notice, this + list of conditions and the following disclaimer. + +2. Redistributions in binary form must reproduce the above copyright notice, + this list of conditions and the following disclaimer in the documentation + and/or other materials provided with the distribution. + +3. Neither the name of the copyright holder nor the names of its + contributors may be used to endorse or promote products derived from + this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +``` diff --git a/app/package.json b/app/package.json index bb62141f..ab5a29b4 100644 --- a/app/package.json +++ b/app/package.json @@ -169,7 +169,7 @@ "$HOME/.copilot", "$HOME/.cursor", "$HOME/.deepseek", - "$HOME/.dsh", + "$HOME/.dsh/sessions", "$HOME/.factory", "$HOME/.forge", "$HOME/.gemini", diff --git a/package.json b/package.json index e137066d..c1288196 100644 --- a/package.json +++ b/package.json @@ -9,6 +9,7 @@ }, "files": [ "dist", + "THIRD_PARTY_NOTICES.md", "!dist/parse-worker.js.map" ], "scripts": { diff --git a/src/providers/dsh.ts b/src/providers/dsh.ts index 60141617..d82664db 100644 --- a/src/providers/dsh.ts +++ b/src/providers/dsh.ts @@ -15,17 +15,25 @@ import type { ProbeRoot, Provider, SessionSource, SessionParser, ParsedProviderC // so node:zlib's one-shot zstdDecompressSync (which decodes a single frame) // must be driven frame-by-frame behind a structural frame-boundary scan. The // scan below is a port of scanZstdFrames from the official -// @deepseek-ai/dsh-session-persistence-jsonl package. +// @deepseek-ai/dsh-session-persistence-jsonl package, which is third-party code +// under its own license - see THIRD_PARTY_NOTICES.md. // zstd landed in node:zlib in 22.15 / 23.8; the package floor is lower, so the // provider degrades with a notice instead of assuming the export exists. -const zstdDecompress = (zlib as { zstdDecompressSync?: (buf: Buffer) => Buffer }).zstdDecompressSync +const zstdDecompress = (zlib as { zstdDecompressSync?: (buf: Buffer, opts?: { maxOutputLength?: number }) => Buffer }).zstdDecompressSync const ZSTD_MAGIC = 0xfd2fb528 // SESSION_FORMAT_VERSION in @deepseek-ai/dsh-session. DSH refuses to load a log // stamped with any other version, and a bump means an event's meaning changed, // so a foreign version is skipped rather than read with today's assumptions. +// A zstd frame's declared content size is attacker-controlled, so a few KB of +// crafted input can expand to gigabytes. Every decode is capped: no single +// frame may exceed this, and no file may decode to more than it would have been +// allowed to occupy uncompressed (MAX_SESSION_FILE_BYTES). Overflow throws, and +// the caller skips the WHOLE file rather than counting the frames it got to. +const MAX_FRAME_DECODED_BYTES = 64 * 1024 * 1024 + const SESSION_FORMAT_VERSION = 0 const MIN_REASONABLE_TIMESTAMP_MS = 1_000_000_000_000 @@ -164,6 +172,13 @@ function mapToolName(raw: string): string { return toolNameMap[raw] ?? raw } +// Usage fields are whatever the JSON held. A string or array would flow +// straight into the global token totals and the persisted cache, where +// `0 + [1, 2]` silently becomes "01,2". Same semantics as copilot.ts. +function numberOrZero(raw: unknown): number { + return typeof raw === 'number' && Number.isFinite(raw) && raw > 0 ? raw : 0 +} + // A log stamped with a version this parser was not written against is skipped // whole: a bump means an event's meaning changed, so reading it with today's // assumptions would report confident wrong numbers. @@ -198,12 +213,24 @@ function projectFromCwd(cwd: string, fallback: string): string { } // Decode every complete frame and yield its JSONL lines. A torn final frame is -// ignored; a structurally corrupt file throws for the caller to report. -function* readZstdLines(buffer: Buffer, maxFrames = Number.POSITIVE_INFINITY): Generator { +// ignored; a structurally corrupt file, or one that decodes past `budget`, +// throws for the caller to report. Exported for the decode-budget test. +export function* readZstdLines( + buffer: Buffer, + maxFrames = Number.POSITIVE_INFINITY, + budget = MAX_SESSION_FILE_BYTES, +): Generator { const { frames } = scanZstdFrames(buffer, maxFrames) + let remaining = budget for (const frame of frames) { - const text = zstdDecompress!(buffer.subarray(frame.start, frame.end)).toString('utf-8') - for (const line of text.split('\n')) { + if (remaining <= 0) throw new Error(`decodes past the ${budget}-byte cap`) + // node throws ERR_BUFFER_TOO_LARGE without allocating past the cap, so the + // per-frame limit doubles as the running budget for the frames after it. + const decoded = zstdDecompress!(buffer.subarray(frame.start, frame.end), { + maxOutputLength: Math.min(remaining, MAX_FRAME_DECODED_BYTES), + }) + remaining -= decoded.length + for (const line of decoded.toString('utf-8').split('\n')) { if (line.trim()) yield line } } @@ -276,7 +303,9 @@ async function readSessionHeader(filePath: string): Promise { return null } } - const text = zstdDecompress(head.subarray(frames[0]!.start, frames[0]!.end)).toString('utf-8') + const text = zstdDecompress(head.subarray(frames[0]!.start, frames[0]!.end), { + maxOutputLength: MAX_FRAME_DECODED_BYTES, + }).toString('utf-8') return text.split('\n').find(l => l.trim()) ?? null } const content = await readSessionFile(filePath) @@ -486,11 +515,11 @@ function createParser(source: SessionSource, seenKeys: Set): SessionPars for (const key of sortedKeys) { const bucket = buckets.get(key)! - const input = bucket.usage.inputTokens ?? 0 - const output = bucket.usage.outputTokens ?? 0 - const cacheRead = bucket.usage.cacheReadTokens ?? 0 - const cacheWrite = bucket.usage.cacheWriteTokens ?? 0 - const reasoning = bucket.usage.reasoningTokens ?? 0 + const input = numberOrZero(bucket.usage.inputTokens) + const output = numberOrZero(bucket.usage.outputTokens) + const cacheRead = numberOrZero(bucket.usage.cacheReadTokens) + const cacheWrite = numberOrZero(bucket.usage.cacheWriteTokens) + const reasoning = numberOrZero(bucket.usage.reasoningTokens) if (input + output + cacheRead + cacheWrite + reasoning === 0) continue const dedupKey = `dsh:${sessionId || source.path}:${key}` diff --git a/tests/providers/dsh.test.ts b/tests/providers/dsh.test.ts index 477bb140..fc2142f9 100644 --- a/tests/providers/dsh.test.ts +++ b/tests/providers/dsh.test.ts @@ -1,10 +1,10 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest' -import { mkdtemp, mkdir, writeFile, readFile, rm } from 'fs/promises' +import { mkdtemp, mkdir, writeFile, readFile, rm, stat } from 'fs/promises' import { join } from 'path' import { homedir, tmpdir } from 'os' import zlib from 'zlib' -import { createDshProvider } from '../../src/providers/dsh.js' +import { createDshProvider, readZstdLines } from '../../src/providers/dsh.js' import { calculateCost } from '../../src/models.js' import type { ParsedProviderCall } from '../../src/providers/types.js' @@ -546,3 +546,75 @@ describe('dsh provider - real log, real container', () => { expect(framed).toHaveLength(2) }) }) + +describe('dsh provider - hostile input', () => { + itZstd('skips a session whose frames decompress to far more than the file cap', async () => { + const dir = join(tmpDir, 'sessions', '--home-u-proj--', 'session-bomb') + await mkdir(dir, { recursive: true }) + const filePath = join(dir, 'session.jsonl.zstd') + // 200 MB of zeros compresses to a few KB. Uncapped this decoded to ~916 MB + // of RSS for a 16 KB file; the per-frame cap now rejects it without + // allocating past the cap. + const bomb = zstdCompress!(Buffer.alloc(200 * 1024 * 1024)) + const good = zstdCompress!(Buffer.from( + sessionHeader({ id: 'session-bomb', cwd: '/home/u/proj' }) + '\n' + + chunkUsage(1, 1, { inputTokens: 100, outputTokens: 10 }, 1786707340000) + '\n', + 'utf-8', + )) + await writeFile(filePath, Buffer.concat([good, bomb])) + expect((await stat(filePath)).size).toBeLessThan(64 * 1024) + + // The whole file is skipped: the frames read before the bomb are not + // counted, so a crafted tail cannot poison a partial total. + expect(await parseAll(createDshProvider(tmpDir), filePath)).toEqual([]) + }) + + itZstd('stops decoding once the frames exceed the running budget', async () => { + const frame = zstdCompress!(Buffer.from('{"type":"turn/start","seq":0,"time":1,"data":{"turn":1}}\n', 'utf-8')) + const buffer = Buffer.concat([frame, frame, frame]) + + expect([...readZstdLines(buffer, Number.POSITIVE_INFINITY, 4096)]).toHaveLength(3) + // A budget under two frames' plaintext stops at the frame that overruns it. + expect(() => [...readZstdLines(buffer, Number.POSITIVE_INFINITY, 60)]).toThrow() + }) + + it('coerces non-numeric usage fields instead of poisoning the totals', async () => { + const filePath = await writePlainSession('--home-u-proj--', 'session-poison', [ + sessionHeader({ id: 'session-poison', cwd: '/home/u/proj' }), + JSON.stringify({ + type: 'assistant/message', seq: 1, time: 1786707340000, + data: { + turn: 1, step: 1, message: { role: 'assistant', content: [] }, + usage: { inputTokens: '999', outputTokens: [1, 2], reasoningTokens: 1e308 * 10, cacheReadTokens: -5, cacheWriteTokens: 7 }, + }, + }), + ]) + + const calls = await parseAll(createDshProvider(tmpDir), filePath) + expect(calls).toHaveLength(1) + // Only the one genuinely numeric field survives; every other shape is 0. + expect(calls[0]).toMatchObject({ + inputTokens: 0, + outputTokens: 0, + reasoningTokens: 0, + cacheReadInputTokens: 0, + cacheCreationInputTokens: 7, + }) + for (const value of [calls[0]!.inputTokens, calls[0]!.outputTokens, calls[0]!.costUSD]) { + expect(typeof value).toBe('number') + expect(Number.isFinite(value)).toBe(true) + } + }) + + it('still skips a call whose usage is all non-numeric', async () => { + const filePath = await writePlainSession('--home-u-proj--', 'session-poison-zero', [ + sessionHeader({ id: 'session-poison-zero', cwd: '/home/u/proj' }), + JSON.stringify({ + type: 'assistant/message', seq: 1, time: 1786707340000, + data: { turn: 1, step: 1, message: { role: 'assistant', content: [] }, usage: { inputTokens: '999', outputTokens: [1, 2] } }, + }), + ]) + + expect(await parseAll(createDshProvider(tmpDir), filePath)).toEqual([]) + }) +})