🐛 fix(server): prevent path traversal in TempFileManager.writeTempFile (#13684)

🐛 fix(server): prevent path traversal in TempFileManager.writeTempFile

Use path.basename() to strip directory components from user-supplied
filenames before writing temp files, preventing arbitrary file write
via crafted filenames like "../../app/startServer.js".

Fixes LOBE-6904

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Arvin Xu 2026-04-09 14:35:20 +08:00 committed by GitHub
parent 475622a4b9
commit b268f44f06
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 54 additions and 2 deletions

View file

@ -10,8 +10,12 @@ vi.mock('node:fs');
vi.mock('node:os');
vi.mock('node:path', () => ({
join: (...args: string[]) => args.join('/'),
basename: (p: string) => p.split('/').pop()!.split('\\').pop()!,
resolve: (...args: string[]) => args.join('/'),
default: {
join: (...args: string[]) => args.join('/'),
basename: (p: string) => p.split('/').pop()!.split('\\').pop()!,
resolve: (...args: string[]) => args.join('/'),
},
}));
@ -92,3 +96,49 @@ describe('TempFileManager', () => {
expect(processOnSpy).toHaveBeenCalledWith('SIGTERM', expect.any(Function));
});
});
// Path traversal regression tests
// These tests use the mocked path module (same as above) but verify that
// basename() is called to strip traversal components before constructing the file path.
describe('TempFileManager - path traversal prevention', () => {
const traversalPayloads = [
{ input: '../../etc/passwd', expected: 'passwd' },
{ input: '../../../tmp/evil.txt', expected: 'evil.txt' },
{ input: '..\\..\\..\\windows\\system32\\evil.dll', expected: 'evil.dll' },
{ input: 'foo/../../bar/evil.txt', expected: 'evil.txt' },
{ input: '../startServer.js', expected: 'startServer.js' },
];
beforeEach(() => {
vi.resetAllMocks();
vi.mocked(tmpdir).mockReturnValue('/tmp');
vi.mocked(mkdtempSync).mockReturnValue('/tmp/test-xyz');
vi.mocked(existsSync).mockReturnValue(true);
});
it.each(traversalPayloads)(
'should sanitize path traversal filename: $input → $expected',
async ({ input, expected }) => {
const manager = new TempFileManager('test-');
const testData = new Uint8Array([0x41, 0x42, 0x43]);
const resultPath = await manager.writeTempFile(testData, input);
// writeFileSync should be called with the safe basename, not the traversal path
expect(writeFileSync).toHaveBeenCalledWith(`/tmp/test-xyz/${expected}`, testData);
expect(resultPath).toBe(`/tmp/test-xyz/${expected}`);
},
);
it('should not write to traversed path', async () => {
const manager = new TempFileManager('test-');
const testData = new Uint8Array([0x41, 0x42, 0x43]);
const resultPath = await manager.writeTempFile(testData, '../../evil.txt');
// Should write to /tmp/test-xyz/evil.txt, NOT /tmp/test-xyz/../../evil.txt
expect(resultPath).toBe('/tmp/test-xyz/evil.txt');
expect(resultPath).not.toContain('..');
expect(writeFileSync).toHaveBeenCalledWith('/tmp/test-xyz/evil.txt', testData);
});
});

View file

@ -1,6 +1,6 @@
import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { basename, join, resolve } from 'node:path';
/**
* Utility class for safely storing temporary files
@ -21,7 +21,9 @@ export class TempFileManager {
*/
async writeTempFile(data: Uint8Array, name: string): Promise<string> {
const filePath = join(this.tempDir, name);
// Sanitize filename to prevent path traversal (GHSA-2g9j-v25c-4j97)
const safeName = basename(name);
const filePath = resolve(this.tempDir, safeName);
try {
writeFileSync(filePath, data);