From 8afca348ffce3f78840e942188e61cd8a9a90224 Mon Sep 17 00:00:00 2001 From: bo0tzz Date: Fri, 17 Apr 2026 19:05:53 +0200 Subject: [PATCH] fix: sanitize filenames before adding to zip (#27893) * fix: sanitize filenames before adding to zip * fix: lints * chore: drop split() --- server/src/services/download.service.spec.ts | 55 ++++++++++++++++++++ server/src/services/download.service.ts | 5 +- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/server/src/services/download.service.spec.ts b/server/src/services/download.service.spec.ts index 1ae1b0b4d8..8b2e2b97f0 100644 --- a/server/src/services/download.service.spec.ts +++ b/server/src/services/download.service.spec.ts @@ -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(), diff --git a/server/src/services/download.service.ts b/server/src/services/download.service.ts index 8d939e9635..3dc9c0dd03 100644 --- a/server/src/services/download.service.ts +++ b/server/src/services/download.service.ts @@ -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}`; }