diff --git a/src/server/utils/__tests__/tempFileManager.test.ts b/src/server/utils/__tests__/tempFileManager.test.ts index ee156947e7..a2629d4129 100644 --- a/src/server/utils/__tests__/tempFileManager.test.ts +++ b/src/server/utils/__tests__/tempFileManager.test.ts @@ -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); + }); +}); diff --git a/src/server/utils/tempFileManager.ts b/src/server/utils/tempFileManager.ts index 81aa75e0c7..a3865b8ee0 100644 --- a/src/server/utils/tempFileManager.ts +++ b/src/server/utils/tempFileManager.ts @@ -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 { - 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);