fix: sanitize filenames before adding to zip (#27893)

* fix: sanitize filenames before adding to zip

* fix: lints

* chore: drop split()
This commit is contained in:
bo0tzz 2026-04-17 19:05:53 +02:00 committed by GitHub
parent 2070f775d6
commit 8afca348ff
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 58 additions and 2 deletions

View File

@ -142,6 +142,61 @@ describe(DownloadService.name, () => {
expect(archiveMock.addFile).toHaveBeenNthCalledWith(2, '/data/library/IMG_123.jpg', 'IMG_123+1.jpg');
});
it.each([
{ input: '../../../../tmp/pwn.jpg', expected: '........tmppwn.jpg' },
{ input: String.raw`C:\temp\abs3.jpg`, expected: 'Ctempabs3.jpg' },
{ input: 'a/../../b.jpg', expected: 'a....b.jpg' },
{ input: String.raw`..\..\win1.jpg`, expected: '....win1.jpg' },
{ input: '/etc/passwd', expected: 'etcpasswd' },
{ input: '..', expected: 'unnamed' },
{ input: '', expected: 'unnamed' },
])('should sanitize unsafe originalFileName "$input" to "$expected"', async ({ input, expected }) => {
const archiveMock = {
addFile: vitest.fn(),
finalize: vitest.fn(),
stream: new Readable(),
};
const asset = AssetFactory.create({ originalFileName: input, originalPath: '/data/library/safe.jpg' });
mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set([asset.id]));
mocks.asset.getForOriginals.mockResolvedValue([asset]);
mocks.storage.createZipStream.mockReturnValue(archiveMock);
await expect(sut.downloadArchive(authStub.admin, { assetIds: [asset.id] })).resolves.toEqual({
stream: archiveMock.stream,
});
expect(archiveMock.addFile).toHaveBeenCalledWith('/data/library/safe.jpg', expected);
});
it('should dedupe sanitized duplicate unsafe filenames', async () => {
const archiveMock = {
addFile: vitest.fn(),
finalize: vitest.fn(),
stream: new Readable(),
};
const asset1 = AssetFactory.create({
originalFileName: '../../../tmp/pwn.jpg',
originalPath: '/data/library/a.jpg',
});
const asset2 = AssetFactory.create({
originalFileName: '../../../tmp/pwn.jpg',
originalPath: '/data/library/b.jpg',
});
mocks.access.asset.checkOwnerAccess.mockResolvedValue(new Set([asset1.id, asset2.id]));
mocks.asset.getForOriginals.mockResolvedValue([asset1, asset2]);
mocks.storage.createZipStream.mockReturnValue(archiveMock);
await expect(sut.downloadArchive(authStub.admin, { assetIds: [asset1.id, asset2.id] })).resolves.toEqual({
stream: archiveMock.stream,
});
expect(archiveMock.addFile).toHaveBeenCalledTimes(2);
expect(archiveMock.addFile).toHaveBeenNthCalledWith(1, '/data/library/a.jpg', '......tmppwn.jpg');
expect(archiveMock.addFile).toHaveBeenNthCalledWith(2, '/data/library/b.jpg', '......tmppwn+1.jpg');
});
it('should resolve symlinks', async () => {
const archiveMock = {
addFile: vitest.fn(),

View File

@ -1,5 +1,6 @@
import { BadRequestException, Injectable } from '@nestjs/common';
import { parse } from 'node:path';
import sanitize from 'sanitize-filename';
import { StorageCore } from 'src/cores/storage.core';
import { AuthDto } from 'src/dtos/auth.dto';
import { DownloadArchiveDto, DownloadArchiveInfo, DownloadInfoDto, DownloadResponseDto } from 'src/dtos/download.dto';
@ -95,11 +96,11 @@ export class DownloadService extends BaseService {
const { originalPath, editedPath, originalFileName } = asset;
let filename = originalFileName;
let filename = sanitize(originalFileName) || 'unnamed';
const count = paths[filename] || 0;
paths[filename] = count + 1;
if (count !== 0) {
const parsedFilename = parse(originalFileName);
const parsedFilename = parse(filename);
filename = `${parsedFilename.name}+${count}${parsedFilename.ext}`;
}