From 1a57c477c9b49739bf2b5af820d27669039f854c Mon Sep 17 00:00:00 2001 From: fergusean Date: Sun, 1 Oct 2023 12:56:49 -0400 Subject: [PATCH] fix(http): prevent JSON payloads from spoofing UploadedFile (#460) * fix(http): prevent JSON payloads from imitating UploadedFile * maybe a better implementation * fix null handling --- packages/http/src/request-parser.ts | 3 +- packages/http/src/router.ts | 14 ++++++++- packages/http/tests/router.spec.ts | 49 +++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 2 deletions(-) diff --git a/packages/http/src/request-parser.ts b/packages/http/src/request-parser.ts index b6f697f4e..9b55950a1 100644 --- a/packages/http/src/request-parser.ts +++ b/packages/http/src/request-parser.ts @@ -16,7 +16,7 @@ import { ValidationError } from '@deepkit/type'; import { BodyValidationError, createRequestWithCachedBody, getRegExp, HttpRequest, ValidatedBody } from './model.js'; -import { RouteConfig, UploadedFile } from './router.js'; +import { RouteConfig, UploadedFile, UploadedFileSymbol } from './router.js'; //@ts-ignore import qs from 'qs'; @@ -40,6 +40,7 @@ function parseBody( for (const [name, file] of Object.entries(files) as any) { if (file.size === 0) continue; foundFiles[name] = { + validator: UploadedFileSymbol, size: file.size, path: file.filepath, name: file.originalFilename, diff --git a/packages/http/src/router.ts b/packages/http/src/router.ts index c46b6e0e8..0c9177106 100644 --- a/packages/http/src/router.ts +++ b/packages/http/src/router.ts @@ -8,7 +8,7 @@ * You should have received a copy of the MIT License along with this program. */ import { ClassType, CompilerContext, getClassName, isArray, isClass, urlJoin } from '@deepkit/core'; -import { entity, ReflectionClass, ReflectionFunction, ReflectionKind, ReflectionParameter, SerializationOptions, Serializer, Type, ValidationError } from '@deepkit/type'; +import { entity, ReflectionClass, ReflectionFunction, ReflectionKind, ReflectionParameter, SerializationOptions, serializer, Serializer, Type, ValidationError } from '@deepkit/type'; import { HttpAction, httpClass, HttpController, HttpDecorator } from './decorator.js'; import { HttpRequest, HttpRequestPositionedParameters, HttpRequestQuery, HttpRequestResolvedParameters } from './model.js'; import { InjectorContext, InjectorModule, TagRegistry } from '@deepkit/injector'; @@ -32,8 +32,15 @@ interface ResolvedController { middlewares?: (injector: InjectorContext) => { fn: HttpMiddlewareFn, timeout?: number }[]; } +export const UploadedFileSymbol = Symbol('UploadedFile'); + @entity.name('@deepkit/UploadedFile') export class UploadedFile { + /** + * Validator to ensure the file was provided by the framework, and not the user spoofing it. + */ + validator!: typeof UploadedFileSymbol | null; + /** * The size of the uploaded file in bytes. */ @@ -66,6 +73,11 @@ export class UploadedFile { // hash!: string | 'sha1' | 'md5' | 'sha256' | null; } +serializer.typeGuards.getRegistry(1).registerClass(UploadedFile, (type, state) => { + state.setContext({ UploadedFileSymbol }); + state.addSetterAndReportErrorIfInvalid('uploadSecurity', 'Not an uploaded file', `typeof ${state.accessor} === 'object' && ${state.accessor} !== null && ${state.accessor}.validator === UploadedFileSymbol`); +}); + export interface RouteFunctionControllerAction { type: 'function'; //if not set, the root module is used diff --git a/packages/http/tests/router.spec.ts b/packages/http/tests/router.spec.ts index 05f4287ea..df7983e02 100644 --- a/packages/http/tests/router.spec.ts +++ b/packages/http/tests/router.spec.ts @@ -1350,3 +1350,52 @@ test('fetch thrown error instances in listeners', async () => { expect((await httpKernel.request(HttpRequest.GET('/'))).statusCode).toEqual(403); }); + +test('upload security', async () => { + class UploadBody { + someFile!: UploadedFile; + } + + class Controller { + @http.POST('/upload') + upload(body: HttpBody): any { + return { uploadedSize: body.someFile.size }; + } + } + + const httpKernel = createHttpKernel([Controller]); + + expect((await httpKernel.request(HttpRequest.POST('/upload').json({ + someFile: { + size: 12345, + path: '/etc/secure-file', + name: 'fakefile', + type: 'image/jpeg', + lastModifiedDate: null + } + }))).json).toMatchObject({ + message: 'Validation error:\nsomeFile(uploadSecurity): Not an uploaded file' + }); + + // ensure type deserialization doesn't set the invalid 'fake value' value to UploadedFileSymbol + expect((await httpKernel.request(HttpRequest.POST('/upload').json({ + someFile: { + validator: 'fake value', + size: 12345, + path: '/etc/secure-file', + name: 'fakefile', + type: 'image/jpeg', + lastModifiedDate: null + } + }))).json).toMatchObject({ + message: 'Validation error:\nsomeFile(uploadSecurity): Not an uploaded file' + }); + + expect((await httpKernel.request(HttpRequest.POST('/upload').multiPart([ + { + name: 'someFile', + file: Buffer.from('testing a text file'), + fileName: 'test.txt' + } + ]))).json).toMatchObject({ uploadedSize: 19 }); +});