From 6fe6b6ec89cfa3c1005ca4cf2219fc77de3fb975 Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Mon, 25 Mar 2024 09:32:53 +0100 Subject: [PATCH] fix: Allow path segments to start with 2 or more dots --- src/storage/mapping/BaseFileIdentifierMapper.ts | 6 +++--- .../unit/storage/mapping/BaseFileIdentifierMapper.test.ts | 8 ++++++-- test/unit/storage/mapping/ExtensionBasedMapper.test.ts | 2 +- test/unit/storage/mapping/FixedContentTypeMapper.test.ts | 2 +- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/storage/mapping/BaseFileIdentifierMapper.ts b/src/storage/mapping/BaseFileIdentifierMapper.ts index c1311d710..5f533b8a5 100644 --- a/src/storage/mapping/BaseFileIdentifierMapper.ts +++ b/src/storage/mapping/BaseFileIdentifierMapper.ts @@ -205,9 +205,9 @@ export class BaseFileIdentifierMapper implements FileIdentifierMapper { throw new BadRequestHttpError('URL needs a / after the base'); } - if (path.includes('/..')) { - this.logger.warn(`Disallowed /.. segment in URL ${identifier.path}.`); - throw new BadRequestHttpError('Disallowed /.. segment in URL'); + if (path.includes('/../') || path.endsWith('/..')) { + this.logger.warn(`Disallowed /../ segment in URL ${identifier.path}.`); + throw new BadRequestHttpError('Disallowed /../ segment in URL'); } } diff --git a/test/unit/storage/mapping/BaseFileIdentifierMapper.test.ts b/test/unit/storage/mapping/BaseFileIdentifierMapper.test.ts index e8fb2ba98..13064d954 100644 --- a/test/unit/storage/mapping/BaseFileIdentifierMapper.test.ts +++ b/test/unit/storage/mapping/BaseFileIdentifierMapper.test.ts @@ -22,9 +22,13 @@ describe('An BaseFileIdentifierMapper', (): void => { }); it('throws 400 if the input path contains relative parts.', async(): Promise => { - const result = mapper.mapUrlToFilePath({ path: `${base}test/../test2` }, false); + let result = mapper.mapUrlToFilePath({ path: `${base}test/../test2` }, false); await expect(result).rejects.toThrow(BadRequestHttpError); - await expect(result).rejects.toThrow('Disallowed /.. segment in URL'); + await expect(result).rejects.toThrow('Disallowed /../ segment in URL'); + + result = mapper.mapUrlToFilePath({ path: `${base}test/..` }, false); + await expect(result).rejects.toThrow(BadRequestHttpError); + await expect(result).rejects.toThrow('Disallowed /../ segment in URL'); }); it('returns the corresponding file path for container identifiers.', async(): Promise => { diff --git a/test/unit/storage/mapping/ExtensionBasedMapper.test.ts b/test/unit/storage/mapping/ExtensionBasedMapper.test.ts index 541739855..758311094 100644 --- a/test/unit/storage/mapping/ExtensionBasedMapper.test.ts +++ b/test/unit/storage/mapping/ExtensionBasedMapper.test.ts @@ -38,7 +38,7 @@ describe('An ExtensionBasedMapper', (): void => { it('throws 400 if the input path contains relative parts.', async(): Promise => { const result = mapper.mapUrlToFilePath({ path: `${base}test/../test2` }, false); await expect(result).rejects.toThrow(BadRequestHttpError); - await expect(result).rejects.toThrow('Disallowed /.. segment in URL'); + await expect(result).rejects.toThrow('Disallowed /../ segment in URL'); }); it('returns the corresponding file path for container identifiers.', async(): Promise => { diff --git a/test/unit/storage/mapping/FixedContentTypeMapper.test.ts b/test/unit/storage/mapping/FixedContentTypeMapper.test.ts index c5ee7a364..6b6f5e6d7 100644 --- a/test/unit/storage/mapping/FixedContentTypeMapper.test.ts +++ b/test/unit/storage/mapping/FixedContentTypeMapper.test.ts @@ -25,7 +25,7 @@ describe('An FixedContentTypeMapper', (): void => { it('throws 400 if the input path contains relative parts.', async(): Promise => { const result = mapper.mapUrlToFilePath({ path: `${base}test/../test2` }, false); await expect(result).rejects.toThrow(BadRequestHttpError); - await expect(result).rejects.toThrow('Disallowed /.. segment in URL'); + await expect(result).rejects.toThrow('Disallowed /../ segment in URL'); }); it('returns the corresponding file path for container identifiers.', async(): Promise => {