diff --git a/apps/server/src/core/attachment/attachment.controller.ts b/apps/server/src/core/attachment/attachment.controller.ts index 7b24cc35..784e527e 100644 --- a/apps/server/src/core/attachment/attachment.controller.ts +++ b/apps/server/src/core/attachment/attachment.controller.ts @@ -53,6 +53,7 @@ import { EnvironmentService } from '../../integrations/environment/environment.s import { TokenService } from '../auth/services/token.service'; import { JwtAttachmentPayload, JwtType } from '../auth/dto/jwt-payload'; import * as path from 'path'; +import { sanitize } from 'sanitize-filename-ts'; import { AttachmentInfoDto, RemoveIconDto } from './dto/attachment.dto'; import { PageAccessService } from '../page/page-access/page-access.service'; import { AuditEvent, AuditResource } from '../../common/events/audit-events'; @@ -356,6 +357,10 @@ export class AttachmentController { throw new BadRequestException('Invalid image attachment type'); } + if (!fileName || sanitize(fileName) !== fileName) { + throw new BadRequestException('Invalid file name'); + } + const filenameWithoutExt = path.basename(fileName, path.extname(fileName)); if (!isValidUUID(filenameWithoutExt)) { throw new BadRequestException('Invalid file id'); diff --git a/apps/server/src/integrations/storage/drivers/local.driver.spec.ts b/apps/server/src/integrations/storage/drivers/local.driver.spec.ts new file mode 100644 index 00000000..ccdddcc5 --- /dev/null +++ b/apps/server/src/integrations/storage/drivers/local.driver.spec.ts @@ -0,0 +1,67 @@ +import { resolve, sep } from 'path'; +import { LocalDriver } from './local.driver'; + +type FullPath = (filePath: string) => string; + +describe('LocalDriver._fullPath', () => { + const ROOT = resolve('/data/storage'); + const driver = new LocalDriver({ storagePath: ROOT }); + const fullPath = ((driver as any)._fullPath as FullPath).bind(driver); + + describe('legitimate inputs (behavior preserved)', () => { + it.each([ + ['workspace-id/avatars/uuid.png', `${ROOT}${sep}workspace-id${sep}avatars${sep}uuid.png`], + ['workspace-id/files/uuid/file.pdf', `${ROOT}${sep}workspace-id${sep}files${sep}uuid${sep}file.pdf`], + ['a/b/c/d/e.bin', `${ROOT}${sep}a${sep}b${sep}c${sep}d${sep}e.bin`], + ['', ROOT], + ['.', ROOT], + ['./x/y.png', `${ROOT}${sep}x${sep}y.png`], + ['a//b', `${ROOT}${sep}a${sep}b`], + ['a/b/../c', `${ROOT}${sep}a${sep}c`], + ])('resolves %j to %j', (input, expected) => { + expect(fullPath(input)).toBe(expected); + }); + }); + + describe('traversal rejected', () => { + it.each([ + '../etc/passwd', + '../../../etc/passwd', + 'workspace/../../../etc/passwd', + '..', + '../..', + 'a/../../..', + ])('throws for %j', (input) => { + expect(() => fullPath(input)).toThrow('Invalid file path'); + }); + }); + + describe('absolute path rejected', () => { + it.each([ + '/etc/passwd', + '/root/.ssh/id_rsa', + sep + 'absolute', + ])('throws for %j', (input) => { + expect(() => fullPath(input)).toThrow('Invalid file path'); + }); + }); + + describe('prefix-confusion rejected', () => { + it('rejects a sibling directory whose name starts with the storage root', () => { + const siblingDriver = new LocalDriver({ storagePath: '/data/storage' }); + const siblingFullPath = ((siblingDriver as any)._fullPath as FullPath).bind(siblingDriver); + // Attempt to reach /data/storage-evil/secret by traversal: + // resolve('/data/storage', '../storage-evil/secret') === '/data/storage-evil/secret' + // Without the `+ sep` guard, a startsWith check would match. + expect(() => siblingFullPath('../storage-evil/secret')).toThrow('Invalid file path'); + }); + }); + + describe('storage root itself', () => { + it('accepts the root when input resolves to it', () => { + expect(fullPath('')).toBe(ROOT); + expect(fullPath('.')).toBe(ROOT); + expect(fullPath('a/..')).toBe(ROOT); + }); + }); +}); diff --git a/apps/server/src/integrations/storage/drivers/local.driver.ts b/apps/server/src/integrations/storage/drivers/local.driver.ts index 39342c61..60be7c5d 100644 --- a/apps/server/src/integrations/storage/drivers/local.driver.ts +++ b/apps/server/src/integrations/storage/drivers/local.driver.ts @@ -3,7 +3,7 @@ import { LocalStorageConfig, StorageOption, } from '../interfaces'; -import { join, dirname } from 'path'; +import { dirname, resolve, sep } from 'path'; import * as fs from 'fs-extra'; import { Readable } from 'stream'; import { createReadStream, createWriteStream } from 'node:fs'; @@ -17,7 +17,12 @@ export class LocalDriver implements StorageDriver { } private _fullPath(filePath: string): string { - return join(this.config.storagePath, filePath); + const storageRoot = resolve(this.config.storagePath); + const fullPath = resolve(storageRoot, filePath); + if (fullPath !== storageRoot && !fullPath.startsWith(storageRoot + sep)) { + throw new Error('Invalid file path'); + } + return fullPath; } async upload(filePath: string, file: Buffer | Readable): Promise {