From de8d6255774c2931a6d7ce28badc7ed07ae72d0e Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Sat, 27 May 2023 14:31:44 +0200 Subject: [PATCH] security: Reject multiple successive slashes in path (#3880) --- src/lib/app.ts | 3 +- .../reject-double-slashes-in-path.ts | 11 ++++ .../leading-slashes-are-stripped.e2e.test.ts | 59 +++++++++++++++++++ 3 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 src/lib/middleware/reject-double-slashes-in-path.ts create mode 100644 src/test/e2e/api/auth/leading-slashes-are-stripped.e2e.test.ts diff --git a/src/lib/app.ts b/src/lib/app.ts index 32db3d0266..346c952641 100644 --- a/src/lib/app.ts +++ b/src/lib/app.ts @@ -29,6 +29,7 @@ import maintenanceMiddleware from './middleware/maintenance-middleware'; import { unless } from './middleware/unless-middleware'; import { catchAllErrorHandler } from './middleware/catch-all-error-handler'; import NotFoundError from './error/notfound-error'; +import { rejectDoubleSlashesInPath } from './middleware/reject-double-slashes-in-path'; export default async function getApp( config: IUnleashConfig, @@ -92,7 +93,7 @@ export default async function getApp( if (config.enableOAS && services.openApiService) { services.openApiService.useDocs(app); } - + app.use(`${baseUriPath}/`, rejectDoubleSlashesInPath); // Support CORS preflight requests for the frontend endpoints. // Preflight requests should not have Authorization headers, // so this must be handled before the API token middleware. diff --git a/src/lib/middleware/reject-double-slashes-in-path.ts b/src/lib/middleware/reject-double-slashes-in-path.ts new file mode 100644 index 0000000000..5d5388ddb0 --- /dev/null +++ b/src/lib/middleware/reject-double-slashes-in-path.ts @@ -0,0 +1,11 @@ +import { RequestHandler } from 'express'; + +const MULTIPLE_SLASHES = /\/\/+/; + +export const rejectDoubleSlashesInPath: RequestHandler = (req, res, next) => { + if (req.path.match(MULTIPLE_SLASHES)) { + res.status(404).send(); + } else { + next(); + } +}; diff --git a/src/test/e2e/api/auth/leading-slashes-are-stripped.e2e.test.ts b/src/test/e2e/api/auth/leading-slashes-are-stripped.e2e.test.ts new file mode 100644 index 0000000000..0feb8533e0 --- /dev/null +++ b/src/test/e2e/api/auth/leading-slashes-are-stripped.e2e.test.ts @@ -0,0 +1,59 @@ +import getLogger from '../../../fixtures/no-logger'; +import dbInit, { ITestDb } from '../../helpers/database-init'; +import { IUnleashTest, setupAppWithAuth } from '../../helpers/test-helper'; +import { IAuthType, IUnleashStores } from '../../../../lib/types'; +import { ApiTokenType } from '../../../../lib/types/models/api-token'; + +let app: IUnleashTest; +let appWithBaseUrl: IUnleashTest; +let stores: IUnleashStores; +let db: ITestDb; + +beforeAll(async () => { + db = await dbInit( + 'multiple_leading_slashes_are_still_authed_serial', + getLogger, + ); + stores = db.stores; + app = await setupAppWithAuth(stores, { + authentication: { enableApiToken: true, type: IAuthType.DEMO }, + }); + appWithBaseUrl = await setupAppWithAuth(stores, { + server: { baseUriPath: '/demo' }, + authentication: { enableApiToken: true, type: IAuthType.DEMO }, + }); +}); + +afterAll(async () => { + await app.destroy(); + await db.destroy(); +}); + +test('Access to /api/client/features are refused no matter how many leading slashes', async () => { + await app.request.get('/api/client/features').expect(401); + await app.request.get('/////api/client/features').expect(404); + await app.request.get('//api/client/features').expect(404); +}); + +test('Multiple slashes anywhere in the path is not a URL that exists', async () => { + await app.request.get('/api/admin///projects/default/features').expect(404); + await app.request.get('/api/client///features').expect(404); +}); + +test('multiple slashes after base path is also rejected with 404', async () => { + await appWithBaseUrl.request.get('/demo///api/client/features').expect(404); + await appWithBaseUrl.request.get('/demo/api/client/features').expect(401); +}); + +test(`Access with API token is granted`, async () => { + let token = await app.services.apiTokenService.createApiTokenWithProjects({ + environment: 'default', + projects: ['default'], + tokenName: 'test', + type: ApiTokenType.CLIENT, + }); + await app.request + .get('/api/client/features') + .set('Authorization', token.secret) + .expect(200); +});