fix(http): prevent JSON payloads from spoofing UploadedFile (#460)

* fix(http): prevent JSON payloads from imitating UploadedFile * maybe a better implementation * fix null handling
deepkit · Oct 1, 2023 · 1a57c47 · 1a57c47
1 parent 74b92fc
commit 1a57c47
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 2 deletions.
diff --git 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
@@ -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
@@ -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<UploadBody>): 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 });
+});