From 8d2413f3e4ee698c89e5496bf2265cff77fd6d17 Mon Sep 17 00:00:00 2001 From: goemen Date: Mon, 23 Sep 2024 17:02:20 -0700 Subject: [PATCH 1/2] fix security issue with file upload path --- backend/src/v1/middlewares/storage/upload.spec.ts | 6 +++++- backend/src/v1/middlewares/storage/upload.ts | 10 +++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/backend/src/v1/middlewares/storage/upload.spec.ts b/backend/src/v1/middlewares/storage/upload.spec.ts index 3be9a2127..b83cf6074 100644 --- a/backend/src/v1/middlewares/storage/upload.spec.ts +++ b/backend/src/v1/middlewares/storage/upload.spec.ts @@ -10,11 +10,13 @@ jest.mock('@aws-sdk/client-s3', () => ({ })), })); +const realpathSyncMock = jest.fn(); jest.mock('fs', () => ({ __esModule: true, ...jest.requireActual('fs'), default: { createReadStream: jest.fn(), + realpathSync: (...args) => realpathSyncMock(...args), }, })); @@ -41,6 +43,8 @@ describe('upload', () => { }, }; + realpathSyncMock.mockReturnValue(path.join(os.tmpdir(), 'test.jpg')); + const res = { status: jest.fn().mockReturnThis(), json: jest.fn(), @@ -83,7 +87,6 @@ describe('upload', () => { }; await useUpload({ folder: 'app' })(req, res, jest.fn()); expect(res.status).toHaveBeenCalledWith(400); - }); }); @@ -100,6 +103,7 @@ describe('upload', () => { }, }; + realpathSyncMock.mockReturnValue(path.join(os.tmpdir(), 'test.jpg')); const res = { status: jest.fn().mockReturnThis(), json: jest.fn(), diff --git a/backend/src/v1/middlewares/storage/upload.ts b/backend/src/v1/middlewares/storage/upload.ts index c6f7e26d5..3b9070bb0 100644 --- a/backend/src/v1/middlewares/storage/upload.ts +++ b/backend/src/v1/middlewares/storage/upload.ts @@ -8,7 +8,7 @@ import { import os from 'os'; import retry from 'async-retry'; import { S3_BUCKET, S3_OPTIONS } from '../../../constants/admin'; - +import PATH from 'path'; interface Options { folder: string; @@ -31,6 +31,14 @@ export const useUpload = (options: Options) => { try { const s3 = new S3Client(S3_OPTIONS); + const filePath = fs.realpathSync(PATH.resolve(os.tmpdir(), path)); + logger.info(`File path: ${filePath}`); + if (!filePath.startsWith(os.tmpdir())) { + logger.error('File path is not starting with temp directory.'); + res.statusCode = 403; + res.end(); + return; + } const stream = fs.createReadStream(path); const uploadParams: PutObjectCommandInput = { Bucket: S3_BUCKET, From be84160c64014d4ccbe07be049cbb938655486b8 Mon Sep 17 00:00:00 2001 From: goemen Date: Mon, 23 Sep 2024 17:25:04 -0700 Subject: [PATCH 2/2] remove logger .info --- backend/src/v1/middlewares/storage/upload.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/backend/src/v1/middlewares/storage/upload.ts b/backend/src/v1/middlewares/storage/upload.ts index 3b9070bb0..4aa7a0fe5 100644 --- a/backend/src/v1/middlewares/storage/upload.ts +++ b/backend/src/v1/middlewares/storage/upload.ts @@ -32,7 +32,6 @@ export const useUpload = (options: Options) => { try { const s3 = new S3Client(S3_OPTIONS); const filePath = fs.realpathSync(PATH.resolve(os.tmpdir(), path)); - logger.info(`File path: ${filePath}`); if (!filePath.startsWith(os.tmpdir())) { logger.error('File path is not starting with temp directory.'); res.statusCode = 403;