diff --git a/src/core.js b/src/core.js index 0748518..8d7ede4 100644 --- a/src/core.js +++ b/src/core.js @@ -134,6 +134,34 @@ export function uniqueSafeFilename(name, existingNames = []) { return candidate; } +function safeZipPath(value) { + const segments = String(value ?? '').replaceAll('\\', '/').split('/'); + const safeSegments = []; + for (const segment of segments) { + if (!segment || segment === '.' || segment === '..') continue; + safeSegments.push(safeFilename(segment)); + } + return safeSegments.join('/') || 'log.txt'; +} + +function uniqueSafeZipPath(value, existingNames = []) { + const safe = safeZipPath(value); + const separator = safe.lastIndexOf('/'); + const directory = separator >= 0 ? safe.slice(0, separator + 1) : ''; + const filename = separator >= 0 ? safe.slice(separator + 1) : safe; + const used = new Set(existingNames.map(name => safeZipPath(name).toLocaleLowerCase('en-US'))); + if (!used.has(safe.toLocaleLowerCase('en-US'))) return safe; + const extensionAt = filename.lastIndexOf('.'); + const hasExtension = extensionAt > 0; + const stem = hasExtension ? filename.slice(0, extensionAt) : filename; + const extension = hasExtension ? filename.slice(extensionAt) : ''; + let index = 2; + let candidate; + do { candidate = `${directory}${stem} (${index++})${extension}`; } + while (used.has(candidate.toLocaleLowerCase('en-US'))); + return candidate; +} + export function isDuplicateLog(files, incoming) { const sourceName = safeFilename(incoming.sourceName || incoming.name).toLocaleLowerCase('en-US'); return files.some(file => safeFilename(file.sourceName || file.name).toLocaleLowerCase('en-US') === sourceName @@ -292,7 +320,7 @@ export function createZip(entries) { let offset = 0; let totalSize = 0; for (const entry of entries) { - const safeName = uniqueSafeFilename(entry.name, [...usedNames]); + const safeName = uniqueSafeZipPath(entry.name, [...usedNames]); usedNames.add(safeName); const name = encoder.encode(safeName); if (name.length > 0xffff) throw new Error('A ZIP filename is too long.'); diff --git a/test/core.test.js b/test/core.test.js index ecbbc79..abccb5c 100644 --- a/test/core.test.js +++ b/test/core.test.js @@ -219,7 +219,7 @@ test('ZIP output contains safe filenames and readable entries', async () => { contents.push(method === 8 ? inflateRawSync(payload) : payload); offset = dataStart + compressedSize; } - assert.deepEqual(names, ['report.md', 'сбой.txt']); + assert.deepEqual(names, ['logs/report.md', 'сбой.txt']); assert.equal(contents[0].toString(), '# Report\nhello'); assert.deepEqual([...contents[1]], [0, 1, 2, 255]); assert.equal(bytes.readUInt32LE(offset), 0x02014b50); @@ -245,10 +245,33 @@ test('ZIP output preserves duplicate basenames as separate entries', async () => contents.push(bytes.subarray(start, start + size).toString()); offset = start + size; } - assert.deepEqual(names, ['latest.log', 'latest (2).log', 'LATEST (3).LOG']); + assert.deepEqual(names, ['logs/latest.log', 'other/latest.log', 'third/LATEST.LOG']); assert.deepEqual(contents, ['first log', 'second log', 'third log']); }); +test('issue ZIP preserves its logs folder and sanitizes unsafe path segments', async () => { + const entries = buildIssuePack({ + report: '# Clean report', + files: [ + { name: 'latest.log', cleaned: 'first log' }, + { name: 'latest.log', cleaned: 'second log' }, + { name: '../../secrets.txt', cleaned: 'third log' }, + ], + }); + const bytes = Buffer.from(await createZip(entries).arrayBuffer()); + const names = []; + let offset = 0; + while (bytes.readUInt32LE(offset) === 0x04034b50) { + const size = bytes.readUInt32LE(offset + 18); + const nameLength = bytes.readUInt16LE(offset + 26); + const extraLength = bytes.readUInt16LE(offset + 28); + names.push(new TextDecoder().decode(bytes.subarray(offset + 30, offset + 30 + nameLength))); + offset += 30 + nameLength + extraLength + size; + } + assert.deepEqual(names, ['bug-report.md', 'logs/latest.log', 'logs/latest (2).log', 'logs/secrets.txt']); + assert.ok(names.every(name => !name.startsWith('/') && !name.split('/').includes('..'))); +}); + test('ZIP32 writer rejects more than 500 files', () => { assert.throws(() => createZip(Array.from({ length: 501 }, () => ({ name: 'x.txt', data: '' }))), /500 files/); });