Skip to content

Commit 1f4b01d

Browse files
claude[bot]claude
andauthored
fix: constrain extractFile reads to the unpacked directory (#451)
readFileWithFd joined the requested filename onto the `<root>.unpacked` directory without checking containment, so a filename containing `..` segments could resolve to a path outside that directory. Route the read through the existing ensureWithin helper (matching readFileSync) so reads are constrained to the unpacked directory and an escaping filename throws. Claude-Session: https://claude.ai/code/session_0155o3ELzXMVngciL3ZsqXgA Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0087cfa commit 1f4b01d

2 files changed

Lines changed: 56 additions & 2 deletions

File tree

src/disk.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,8 @@ export function readFileWithFd(
412412
return buffer;
413413
}
414414
if (info.unpacked) {
415-
buffer = fs.readFileSync(path.join(`${filesystem.getRootPath()}.unpacked`, filename));
415+
const unpackedDir = `${filesystem.getRootPath()}.unpacked`;
416+
buffer = fs.readFileSync(ensureWithin(unpackedDir, filename));
416417
} else {
417418
const offset = 8 + filesystem.getHeaderSize() + parseInt(info.offset);
418419
fs.readSync(fd, buffer, 0, info.size, offset);

test/disk-spec.ts

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
readArchiveHeaderSync,
88
readFilesystemSync,
99
readFileSync,
10+
readFileWithFd,
1011
uncacheFilesystem,
1112
} from '../src/disk.js';
1213
import { useTmpDir } from './util/tmpDir.js';
@@ -72,7 +73,7 @@ function makeFileInfo(offset: string, size: number): FilesystemFileEntry {
7273
}
7374

7475
describe('disk', () => {
75-
const { testRunDir, createFixture } = useTmpDir(uncacheAll);
76+
const { testRunDir, tmpDir, createFixture } = useTmpDir(uncacheAll);
7677

7778
describe('caching', () => {
7879
it('uncacheFilesystem should return true for cached and false for uncached', async () => {
@@ -266,4 +267,56 @@ describe('disk', () => {
266267
expect(result.toString('utf8')).toBe(content);
267268
});
268269
});
270+
271+
describe('unpacked file reads', () => {
272+
function makeUnpackedInfo(size: number): FilesystemFileEntry {
273+
return {
274+
offset: '0',
275+
size,
276+
unpacked: true,
277+
executable: false,
278+
integrity: { hash: '', algorithm: 'SHA256', blocks: [], blockSize: 0 },
279+
};
280+
}
281+
282+
/**
283+
* Sets up an archive whose `.unpacked` directory contains a benign file,
284+
* plus an `outside.txt` file that lives next to the archive (outside the
285+
* `.unpacked` directory). Returns the filesystem and the on-disk paths.
286+
*/
287+
function setupUnpacked(name: string) {
288+
const baseDir = tmpDir(name);
289+
const archivePath = path.join(baseDir, `${name}.asar`);
290+
const unpackedDir = `${archivePath}.unpacked`;
291+
fs.mkdirpSync(unpackedDir);
292+
fs.writeFileSync(path.join(unpackedDir, 'inside.txt'), 'inside');
293+
fs.writeFileSync(path.join(baseDir, 'outside.txt'), 'outside');
294+
const filesystem = makeFilesystem(archivePath, 16);
295+
return { filesystem, archivePath };
296+
}
297+
298+
it('readFileSync reads unpacked files within the unpacked directory', () => {
299+
const { filesystem } = setupUnpacked('unpacked-read-sync');
300+
const info = makeUnpackedInfo('inside'.length);
301+
expect(readFileSync(filesystem, 'inside.txt', info).toString('utf8')).toBe('inside');
302+
});
303+
304+
it('readFileSync throws when filename escapes the unpacked directory', () => {
305+
const { filesystem } = setupUnpacked('unpacked-escape-sync');
306+
const info = makeUnpackedInfo('outside'.length);
307+
expect(() => readFileSync(filesystem, '../outside.txt', info)).toThrow('outside');
308+
});
309+
310+
it('readFileWithFd reads unpacked files within the unpacked directory', () => {
311+
const { filesystem } = setupUnpacked('unpacked-read-fd');
312+
const info = makeUnpackedInfo('inside'.length);
313+
expect(readFileWithFd(-1, filesystem, 'inside.txt', info).toString('utf8')).toBe('inside');
314+
});
315+
316+
it('readFileWithFd throws when filename escapes the unpacked directory', () => {
317+
const { filesystem } = setupUnpacked('unpacked-escape-fd');
318+
const info = makeUnpackedInfo('outside'.length);
319+
expect(() => readFileWithFd(-1, filesystem, '../outside.txt', info)).toThrow('outside');
320+
});
321+
});
269322
});

0 commit comments

Comments
 (0)