diff --git a/.changeset/fix-windows-tarball-path-traversal.md b/.changeset/fix-windows-tarball-path-traversal.md new file mode 100644 index 0000000000..665dde9b1e --- /dev/null +++ b/.changeset/fix-windows-tarball-path-traversal.md @@ -0,0 +1,6 @@ +--- +"@pnpm/store.cafs": patch +"pnpm": patch +--- + +Fixed a path traversal vulnerability in tarball extraction on Windows. The path normalization was only checking for `./` but not `.\`. Since backslashes are directory separators on Windows, malicious packages could use paths like `foo\..\..\.npmrc` to write files outside the package directory. diff --git a/store/cafs/src/parseTarball.ts b/store/cafs/src/parseTarball.ts index 84e88b4635..4098194698 100644 --- a/store/cafs/src/parseTarball.ts +++ b/store/cafs/src/parseTarball.ts @@ -104,9 +104,10 @@ export function parseTarball (buffer: Buffer): IParseResult { } } - if (fileName.includes('./')) { - // Bizarre edge case - fileName = path.posix.join('/', fileName).slice(1) + if (fileName.includes('./') || fileName.includes('.\\')) { + // Normalize path traversal attempts (including Windows backslash traversal) + // Replaces backslashes with forward slashes and uses POSIX path normalization to resolve .. + fileName = path.posix.join('/', fileName.replaceAll('\\', '/')).slice(1) } // Values '\0' and '0' are normal files. diff --git a/store/cafs/test/index.ts b/store/cafs/test/index.ts index 0646a4ac6b..0f5e8dea97 100644 --- a/store/cafs/test/index.ts +++ b/store/cafs/test/index.ts @@ -8,6 +8,7 @@ import { checkPkgFilesIntegrity, getFilePathByModeInCafs, } from '../src/index.js' +import { parseTarball } from '../src/parseTarball.js' const f = fixtures(__dirname) @@ -145,6 +146,76 @@ test('unpack a tarball that contains hard links', () => { expect(Object.keys(filesIndex).length).toBeGreaterThan(0) }) +// Regression test for Windows path traversal vulnerability +// A malicious tarball entry like "foo\..\..\..\.npmrc" should have its path normalized +test('path traversal with backslashes is blocked (Windows security fix)', () => { + // Create a minimal valid tarball with a malicious filename + const tarBuffer = createTarballWithEntry('foo\\..\\..\\..\\malicious.txt', 'evil content') + + const result = parseTarball(tarBuffer) + const fileNames = Array.from(result.files.keys()) + + // The path should be normalized - no ".." segments and no path traversal + for (const fileName of fileNames) { + expect(fileName).not.toContain('..') + expect(fileName).not.toContain('\\') + } +}) + +// Helper to create a minimal tarball buffer with a single entry +function createTarballWithEntry (fileName: string, content: string): Buffer { + const contentBytes = Buffer.from(content, 'utf8') + + // Create a 512-byte header + const header = Buffer.alloc(512, 0) + + // File name at offset 0 (max 100 chars) + const nameToWrite = `package/${fileName}` + header.write(nameToWrite, 0, Math.min(nameToWrite.length, 100), 'utf8') + + // File mode at offset 100 (octal, 8 bytes) - 0644 + header.write('0000644\0', 100, 8, 'utf8') + + // UID at offset 108 (octal, 8 bytes) + header.write('0000000\0', 108, 8, 'utf8') + + // GID at offset 116 (octal, 8 bytes) + header.write('0000000\0', 116, 8, 'utf8') + + // File size at offset 124 (octal, 12 bytes) + const sizeOctal = contentBytes.length.toString(8).padStart(11, '0') + header.write(sizeOctal + '\0', 124, 12, 'utf8') + + // Mtime at offset 136 (octal, 12 bytes) + header.write('00000000000\0', 136, 12, 'utf8') + + // File type at offset 156 ('0' for regular file) + header[156] = '0'.charCodeAt(0) + + // USTAR indicator at offset 257 + header.write('ustar\0', 257, 6, 'utf8') + header.write('00', 263, 2, 'utf8') + + // Compute checksum (offset 148, 8 bytes) - sum of all header bytes treating checksum field as spaces + // First, fill checksum field with spaces + header.fill(' ', 148, 156) + let checksum = 0 + for (let i = 0; i < 512; i++) { + checksum += header[i] + } + const checksumOctal = checksum.toString(8).padStart(6, '0') + header.write(checksumOctal + '\0 ', 148, 8, 'utf8') + + // Content block (padded to 512 bytes) + const contentBlock = Buffer.alloc(512, 0) + contentBytes.copy(contentBlock) + + // End-of-archive marker (two 512-byte blocks of zeros) + const endMarker = Buffer.alloc(1024, 0) + + return Buffer.concat([header, contentBlock, endMarker]) +} + // Related issue: https://github.com/pnpm/pnpm/issues/7120 test('unpack should not fail when the tarball format seems to be not USTAR or GNU TAR', () => { const dest = tempy.directory()