From aeefb962d41f82305637cedfcc364732ae65336d Mon Sep 17 00:00:00 2001 From: bo0tzz Date: Tue, 20 Oct 2026 15:39:49 +0200 Subject: [PATCH] fix(server): sanitize EXIF template variables and tighten storage path check (#32566) (cherry picked from commit 6141f8bdbf6d21a14558d9f7d7325918023d1037) --- .../services/storage-template.service.spec.ts | 39 +++++++++++++++++++ .../src/services/storage-template.service.ts | 16 +++++--- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/server/src/services/storage-template.service.spec.ts b/server/src/services/storage-template.service.spec.ts index 072a18c88..5ea6b673c 100644 --- a/server/src/services/storage-template.service.spec.ts +++ b/server/src/services/storage-template.service.spec.ts @@ -237,6 +237,45 @@ describe(StorageTemplateService.name, () => { }); }); + it('should sanitize the make, model and lensModel variables', async () => { + const user = UserFactory.create(); + const asset = AssetFactory.from() + .owner(user) + .exif({ make: '../../sibling', model: 'a/b', lensModel: 'XF27mm F2.8' }) + .build(); + const config = structuredClone(defaults); + config.storageTemplate.template = '{{make}}/{{model}}/{{lensModel}}/{{filename}}'; + sut.onConfigInit({ newConfig: config }); + + mocks.user.get.mockResolvedValue(user); + mocks.assetJob.getForStorageTemplateJob.mockResolvedValueOnce(getForStorageTemplate(asset)); + + expect(await sut.handleMigrationSingle({ id: asset.id })).toBe(JobStatus.Success); + + expect(mocks.move.create).toHaveBeenCalledWith({ + entityId: asset.id, + newPath: `/data/library/${user.id}/....sibling/ab/XF27mm F2.8/${asset.originalFileName}`, + oldPath: asset.originalPath, + pathType: AssetPathType.Original, + }); + }); + + it('should not move an asset to a sibling of the library folder', async () => { + const user = UserFactory.create(); + const asset = AssetFactory.from().owner(user).exif().build(); + const config = structuredClone(defaults); + config.storageTemplate.template = '../{{filename}}'; + sut.onConfigInit({ newConfig: config }); + + mocks.user.get.mockResolvedValue(user); + mocks.assetJob.getForStorageTemplateJob.mockResolvedValueOnce(getForStorageTemplate(asset)); + + expect(await sut.handleMigrationSingle({ id: asset.id })).toBe(JobStatus.Success); + + expect(mocks.move.create).not.toHaveBeenCalled(); + expect(mocks.asset.update).not.toHaveBeenCalled(); + }); + it('should use handlebar else condition for album', async () => { const user = UserFactory.create(); const asset = AssetFactory.from().owner(user).exif().build(); diff --git a/server/src/services/storage-template.service.ts b/server/src/services/storage-template.service.ts index 1b135f0fa..79299ac16 100644 --- a/server/src/services/storage-template.service.ts +++ b/server/src/services/storage-template.service.ts @@ -344,8 +344,14 @@ export class StorageTemplateService extends BaseService { const fullPath = path.normalize(path.join(rootPath, storagePath)); let destination = `${fullPath}.${extension}`; - if (!fullPath.startsWith(rootPath)) { - this.logger.warn(`Skipped attempt to access an invalid path: ${fullPath}. Path should start with ${rootPath}`); + const relativePath = path.relative(rootPath, fullPath); + if ( + !relativePath || + relativePath === '..' || + relativePath.startsWith(`..${path.sep}`) || + path.isAbsolute(relativePath) + ) { + this.logger.warn(`Skipped attempt to access an invalid path: ${fullPath}. Path should be inside ${rootPath}`); return source; } @@ -414,9 +420,9 @@ export class StorageTemplateService extends BaseService { assetIdShort: asset.id.slice(-12), //just throw into the root if it doesn't belong to an album album: (albumName && sanitize(albumName.replaceAll(/\.+/g, ''))) || '', - make: make ?? '', - model: model ?? '', - lensModel: lensModel ?? '', + make: sanitize(make ?? ''), + model: sanitize(model ?? ''), + lensModel: sanitize(lensModel ?? ''), }; const dt = DateTime.fromJSDate(asset.fileCreatedAt);