From 0560be0830f7d73e1bbdda442be8d3d4be359759 Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Sat, 8 Aug 2026 22:23:10 +0000 Subject: [PATCH] feat(storage): reject uploads whose extension, declared type and bytes disagree Both managed lanes now enforce declared MIME == magic bytes == filename extension, so an HTML payload named .jpg is refused rather than stored under a type it is not. --- .../__tests__/confirm-upload.test.ts | 103 +++++++++ .../__tests__/s3-signer.integration.test.ts | 37 ++++ .../package.json | 3 +- .../src/confirm-upload.ts | 91 ++++++++ .../src/index.ts | 8 +- .../src/plugin.ts | 12 ++ .../src/s3-signer.ts | 45 ++++ .../__tests__/upload-resolver.test.ts | 54 +++++ graphile/graphile-settings/package.json | 1 + .../graphile-settings/src/upload-resolver.ts | 20 ++ pnpm-lock.yaml | 6 + .../__tests__/type-agreement.test.ts | 118 ++++++++++ uploads/mime-bytes/src/index.ts | 9 + uploads/mime-bytes/src/type-agreement.ts | 202 ++++++++++++++++++ 14 files changed, 707 insertions(+), 2 deletions(-) create mode 100644 graphile/graphile-presigned-url-plugin/__tests__/confirm-upload.test.ts create mode 100644 graphile/graphile-presigned-url-plugin/src/confirm-upload.ts create mode 100644 uploads/mime-bytes/__tests__/type-agreement.test.ts create mode 100644 uploads/mime-bytes/src/type-agreement.ts diff --git a/graphile/graphile-presigned-url-plugin/__tests__/confirm-upload.test.ts b/graphile/graphile-presigned-url-plugin/__tests__/confirm-upload.test.ts new file mode 100644 index 0000000000..a6f4f710d3 --- /dev/null +++ b/graphile/graphile-presigned-url-plugin/__tests__/confirm-upload.test.ts @@ -0,0 +1,103 @@ +/** + * The presigned lane's byte check: what a `requested` row becomes once the + * client's PUT is supposed to have landed. + * + * S3 is mocked at the client boundary, and the assertions are about the verdict — + * `uploaded`, `rejected` or `expired` — since the transition itself belongs to + * the generated SQL functions, not to this module. + */ + +import { Readable } from 'stream'; + +import { confirmUploadedBytes } from '../src/confirm-upload'; +import type { S3Config } from '../src/types'; + +const PNG = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d]); +const HTML = Buffer.from('\n'); + +/** An S3 whose object is `body`, or absent when `body` is null. */ +function fakeS3(body: Buffer | null): { s3: S3Config; send: jest.Mock } { + const send = jest.fn().mockImplementation(async () => { + if (body === null) { + const err: any = new Error('NoSuchKey'); + err.name = 'NoSuchKey'; + throw err; + } + return { Body: Readable.from([body]) }; + }); + return { s3: { client: { send } as any, bucket: 'myapp-default-public-db' }, send }; +} + +describe('confirmUploadedBytes', () => { + it('confirms an object whose bytes match its claims', async () => { + const { s3, send } = fakeS3(PNG); + + const verdict = await confirmUploadedBytes({ + s3, + key: 'abc', + declaredMime: 'image/png', + filename: 'avatar.png', + }); + + expect(verdict).toEqual({ outcome: 'uploaded', detectedMime: 'image/png' }); + // A ranged read, not a download: the whole point of validating in confirm. + expect(send.mock.calls[0][0].input.Range).toMatch(/^bytes=0-/); + }); + + it('rejects HTML uploaded as a JPEG', async () => { + const { s3 } = fakeS3(HTML); + + const verdict = await confirmUploadedBytes({ + s3, + key: 'abc', + declaredMime: 'image/jpeg', + filename: 'avatar.jpg', + }); + + expect(verdict.outcome).toBe('rejected'); + if (verdict.outcome === 'rejected') { + expect(verdict.reason).toContain('image/jpeg'); + } + }); + + it('rejects an empty object, which was written but is not a file', async () => { + const { s3 } = fakeS3(Buffer.alloc(0)); + + const verdict = await confirmUploadedBytes({ + s3, + key: 'abc', + declaredMime: 'image/png', + filename: 'avatar.png', + }); + + expect(verdict.outcome).toBe('rejected'); + }); + + it('expires rather than rejects when the client never uploaded', async () => { + const { s3 } = fakeS3(null); + + const verdict = await confirmUploadedBytes({ + s3, + key: 'abc', + declaredMime: 'image/png', + filename: 'avatar.png', + }); + + expect(verdict.outcome).toBe('expired'); + }); + + it('confirms a file whose declared type the bytes cannot refute', async () => { + // An unrecognised binary format detects as nothing; silence is not a + // contradiction, so a legitimate upload is not held hostage to the registry. + const { s3 } = fakeS3(Buffer.from([0x00, 0x01, 0x02, 0x03, 0x00, 0x00, 0x00, 0x00])); + + const verdict = await confirmUploadedBytes({ + s3, + key: 'abc', + declaredMime: 'application/vnd.myapp.thing', + filename: 'data.myapp', + }); + + expect(verdict.outcome).toBe('uploaded'); + }); +}); diff --git a/graphile/graphile-presigned-url-plugin/__tests__/s3-signer.integration.test.ts b/graphile/graphile-presigned-url-plugin/__tests__/s3-signer.integration.test.ts index 7fee7273a5..2349275ba8 100644 --- a/graphile/graphile-presigned-url-plugin/__tests__/s3-signer.integration.test.ts +++ b/graphile/graphile-presigned-url-plugin/__tests__/s3-signer.integration.test.ts @@ -17,6 +17,7 @@ import { generatePresignedGetUrl, generatePresignedPutUrl, headObject, + readObjectPrefix, } from '../src/s3-signer'; import type { S3Config } from '../src/types'; @@ -183,6 +184,42 @@ describe('s3-signer integration (MinIO)', () => { }); }); + describe('readObjectPrefix', () => { + const PREFIX_KEY = 'test-read-prefix.bin'; + // Longer than the range this reads, so a truncated response proves the range + // was honoured rather than the whole object downloaded. + const PREFIX_CONTENT = 'ABCDEFGHIJ'.repeat(100); + + beforeAll(async () => { + const putUrl = await generatePresignedPutUrl( + s3Config, + PREFIX_KEY, + 'application/octet-stream', + Buffer.byteLength(PREFIX_CONTENT), + ); + const res = await uploadToPresignedUrl(putUrl, PREFIX_CONTENT, 'application/octet-stream'); + if (res.status !== 200) throw new Error(`Setup upload failed: ${res.status}`); + }); + + it('should read only the requested leading bytes', async () => { + const prefix = await readObjectPrefix(s3Config, PREFIX_KEY, 10); + + expect(prefix).not.toBeNull(); + expect(prefix!.length).toBe(10); + expect(prefix!.toString()).toBe('ABCDEFGHIJ'); + }); + + it('should return the whole object when it is shorter than the range', async () => { + const prefix = await readObjectPrefix(s3Config, PREFIX_KEY, 100000); + expect(prefix!.length).toBe(Buffer.byteLength(PREFIX_CONTENT)); + }); + + it('should return null for a non-existent object', async () => { + const prefix = await readObjectPrefix(s3Config, 'does-not-exist-' + Date.now(), 32); + expect(prefix).toBeNull(); + }); + }); + describe('generatePresignedGetUrl', () => { const GET_KEY = 'test-get-download.txt'; const GET_CONTENT = 'Downloadable content for presigned GET test'; diff --git a/graphile/graphile-presigned-url-plugin/package.json b/graphile/graphile-presigned-url-plugin/package.json index a97a6a3f25..b12c165823 100644 --- a/graphile/graphile-presigned-url-plugin/package.json +++ b/graphile/graphile-presigned-url-plugin/package.json @@ -44,7 +44,8 @@ "@aws-sdk/s3-request-presigner": "^3.1052.0", "@pgpmjs/logger": "workspace:^", "@pgsql/quotes": "^18.2.4", - "lru-cache": "^11.2.7" + "lru-cache": "^11.2.7", + "mime-bytes": "workspace:^" }, "peerDependencies": { "grafast": "^1.1.1", diff --git a/graphile/graphile-presigned-url-plugin/src/confirm-upload.ts b/graphile/graphile-presigned-url-plugin/src/confirm-upload.ts new file mode 100644 index 0000000000..415ea6d6fa --- /dev/null +++ b/graphile/graphile-presigned-url-plugin/src/confirm-upload.ts @@ -0,0 +1,91 @@ +/** + * The byte-validating half of the presigned lane's confirmation. + * + * A presigned upload's bytes never pass through the server: the client PUTs them + * straight to S3, so at the moment the files row is written the only statements + * about the file are the client's own. `requested` is exactly that state — a + * claim — and the row must not reach `uploaded` until the bytes behind it have + * been looked at, because every downstream processing step (image versions, + * extraction, embeddings) fires on `uploaded` and would otherwise be handed + * whatever the client chose to upload. + * + * So confirmation answers three questions in order, and each has a distinct + * outcome on the row: + * + * 1. did the bytes arrive? no → `expired` (the client walked away) + * 2. are they the file they claim? no → `rejected` (and the object deleted) + * 3. otherwise → `uploaded` + * + * The bytes are read with a ranged GET of the leading bytes, not downloaded: a + * magic-byte signature is in the first few dozen bytes, so this costs the same + * for a 2GB video as for an icon. + * + * This module is the decision, not the transition. The worker that owns the + * `storage:confirm_upload` job applies it by calling the generated + * `_confirm_uploaded` / `_reject_file` / `_expire_file` functions — the + * verdict is returned rather than executed so that the same rule can be applied + * by any transport, and tested without a database. + */ + +import { detectFromBuffer } from 'mime-bytes'; +import { checkTypeAgreement } from 'mime-bytes'; + +import { readObjectPrefix } from './s3-signer'; +import type { S3Config } from './types'; + +/** + * How many leading bytes to read. Signatures are far shorter than this; the + * margin covers formats whose signature sits at an offset (e.g. the `ftyp` box + * of an MP4) and gives charset detection enough text to work with. + */ +export const CONFIRM_PREFIX_BYTES = 4096; + +export interface ConfirmUploadInput { + s3: S3Config; + /** The object key the presigned PUT was signed for. */ + key: string; + /** The MIME type the client declared when the row was created. */ + declaredMime: string; + /** The filename recorded on the files row, if any. */ + filename?: string | null; +} + +export type ConfirmUploadVerdict = + | { outcome: 'uploaded'; detectedMime: string | null } + | { outcome: 'rejected'; reason: string; detectedMime: string | null } + | { outcome: 'expired'; reason: string }; + +/** + * Decide what should happen to a `requested` files row, from its object's bytes. + * + * A missing object is `expired` rather than `rejected`: nothing was uploaded, so + * there is nothing to reject, and the row's own retry/expiry budget governs how + * long the client has left. An empty object, by contrast, *was* written and is + * not a file. + */ +export async function confirmUploadedBytes(input: ConfirmUploadInput): Promise { + const { s3, key, declaredMime, filename } = input; + + const prefix = await readObjectPrefix(s3, key, CONFIRM_PREFIX_BYTES); + + if (prefix === null) { + return { outcome: 'expired', reason: `no object at key ${key}: the upload never arrived` }; + } + if (prefix.length === 0) { + return { + outcome: 'rejected', + reason: `object at key ${key} is empty; an upload must carry at least one byte`, + detectedMime: null, + }; + } + + const detected = await detectFromBuffer(prefix); + const detectedMime = detected?.mimeType ?? null; + + const agreement = checkTypeAgreement({ filename, declaredMime, detectedMime }); + if (!agreement.ok) { + return { outcome: 'rejected', reason: agreement.violation.message, detectedMime }; + } + + return { outcome: 'uploaded', detectedMime }; +} diff --git a/graphile/graphile-presigned-url-plugin/src/index.ts b/graphile/graphile-presigned-url-plugin/src/index.ts index dbaf92d28c..bba54f44a2 100644 --- a/graphile/graphile-presigned-url-plugin/src/index.ts +++ b/graphile/graphile-presigned-url-plugin/src/index.ts @@ -27,6 +27,12 @@ * ``` */ +export { + CONFIRM_PREFIX_BYTES, + confirmUploadedBytes, + type ConfirmUploadInput, + type ConfirmUploadVerdict, +} from './confirm-upload'; export type { ResolvedBucketCoordinate } from './default-bucket'; export { resolveDefaultBucket } from './default-bucket'; export { createDownloadUrlPlugin } from './download-url-field'; @@ -44,7 +50,7 @@ export { mintPhysicalBucketName, provisionAndRecordPhysicalBucket, resolveS3, re export { createPresignedUrlPlugin,PresignedUrlPlugin } from './plugin'; export { PresignedUrlPreset } from './preset'; export { type WithPgClient, withRequestPgClient } from './request-pg-client'; -export { copyS3Object, deleteS3Object, generatePresignedGetUrl, generatePresignedPutUrl, headObject } from './s3-signer'; +export { copyS3Object, deleteS3Object, generatePresignedGetUrl, generatePresignedPutUrl, headObject, readObjectPrefix } from './s3-signer'; export { clearBucketCache, clearStorageModuleCache, getBucketConfig, getStorageModuleConfig, getStorageModuleConfigForOwner, isS3BucketProvisioned, loadAllStorageModules, markS3BucketProvisioned,resolveStorageConfigFromCodec, resolveStorageModuleByFileId } from './storage-module-cache'; export type { BucketConfig, diff --git a/graphile/graphile-presigned-url-plugin/src/plugin.ts b/graphile/graphile-presigned-url-plugin/src/plugin.ts index 7921175a2c..00b9b54de4 100644 --- a/graphile/graphile-presigned-url-plugin/src/plugin.ts +++ b/graphile/graphile-presigned-url-plugin/src/plugin.ts @@ -22,6 +22,7 @@ import 'graphile-build'; import { Logger } from '@pgpmjs/logger'; import { access, context as grafastContext, lambda, object } from 'grafast'; import type { GraphileConfig } from 'graphile-config'; +import { checkTypeAgreement } from 'mime-bytes'; import { resolveDefaultBucket } from './default-bucket'; import { buildFileProjection, type FileProjection } from './managed-upload'; @@ -653,6 +654,17 @@ async function processSingleFile( } } + // The bytes are not here to be examined — the client PUTs them straight to S3 — + // so this checks the two claims that *are* here against each other. It is the + // cheap half of the rule: an upload declaring `image/jpeg` under the name + // `payload.html` is refused before a row exists, without reading a byte. The + // bytes themselves are checked on confirmation, before the row leaves + // `requested`. + const agreement = checkTypeAgreement({ filename, declaredMime: contentType }); + if (!agreement.ok) { + throw new Error(`UPLOAD_TYPE_MISMATCH: ${agreement.violation.message}`); + } + // Validate content type against bucket's allowed_mime_types if (bucket.allowed_mime_types && bucket.allowed_mime_types.length > 0) { const allowed = bucket.allowed_mime_types as string[]; diff --git a/graphile/graphile-presigned-url-plugin/src/s3-signer.ts b/graphile/graphile-presigned-url-plugin/src/s3-signer.ts index 6b26745b11..699729560c 100644 --- a/graphile/graphile-presigned-url-plugin/src/s3-signer.ts +++ b/graphile/graphile-presigned-url-plugin/src/s3-signer.ts @@ -130,6 +130,51 @@ export async function copyS3Object( log.debug(`Copied S3 object: bucket=${s3Config.bucket}, ${sourceKey} → ${destinationKey}`); } +/** + * Read the leading bytes of an object. + * + * A ranged GET, because the only reason to touch bytes the client uploaded + * directly is to see what they actually are: a magic-byte signature lives in the + * first few dozen bytes, so validating a 2GB video costs the same as validating + * an icon. + * + * Returns null when the object is not there — the presigned lane's ordinary + * "client never PUT it" case, which is an expiry rather than a failure. + * + * @param s3Config - S3 client and bucket configuration + * @param key - S3 object key + * @param byteCount - How many leading bytes to read + */ +export async function readObjectPrefix( + s3Config: S3Config, + key: string, + byteCount: number, +): Promise { + try { + const response = await s3Config.client.send( + new GetObjectCommand({ + Bucket: s3Config.bucket, + Key: key, + Range: `bytes=0-${byteCount - 1}`, + }), + ); + + const body = response.Body as unknown as AsyncIterable | undefined; + if (!body) return Buffer.alloc(0); + + const chunks: Buffer[] = []; + for await (const chunk of body) { + chunks.push(Buffer.from(chunk)); + } + return Buffer.concat(chunks); + } catch (e: any) { + if (e.name === 'NoSuchKey' || e.name === 'NotFound' || e.$metadata?.httpStatusCode === 404) { + return null; + } + throw e; + } +} + /** * Check if an object exists in S3 and optionally verify its content-type. * diff --git a/graphile/graphile-settings/__tests__/upload-resolver.test.ts b/graphile/graphile-settings/__tests__/upload-resolver.test.ts index e3e1155e94..62cfe464f2 100644 --- a/graphile/graphile-settings/__tests__/upload-resolver.test.ts +++ b/graphile/graphile-settings/__tests__/upload-resolver.test.ts @@ -318,4 +318,58 @@ describe('multipart upload resolver', () => { ), ).rejects.toThrow('UPLOAD_FIELD_UNKNOWN'); }); + + it('rejects an extension that disagrees with the bytes, before anything is written', async () => { + // The html-as-jpg attack: a browser fetching this as an image would execute + // the script in it. + const { constructiveUploadFieldDefinitions, mockUploadWithContentType } = + await loadUploadResolverModule({ detectedContentType: 'text/html' }); + const { context, queries } = fakeContext(); + + await expect( + definitionFor(constructiveUploadFieldDefinitions, 'image').resolve( + makeFakeUpload('avatar.jpg') as any, + {}, + context, + { uploadPlugin: { tags: {}, type: 'image', field: FIELD } }, + ), + ).rejects.toThrow('UPLOAD_TYPE_MISMATCH'); + + expect(mockUploadWithContentType).not.toHaveBeenCalled(); + expect(queries.some((q) => /INSERT/.test(q.text))).toBe(false); + }); + + it('rejects a declared MIME type that disagrees with the bytes', async () => { + const { constructiveUploadFieldDefinitions, mockUploadWithContentType } = + await loadUploadResolverModule({ detectedContentType: 'application/pdf' }); + const { context } = fakeContext(); + + await expect( + definitionFor(constructiveUploadFieldDefinitions, 'upload').resolve( + { ...makeFakeUpload('report.pdf'), mimetype: 'image/png' } as any, + {}, + context, + { uploadPlugin: { tags: {}, type: 'upload', field: FIELD } }, + ), + ).rejects.toThrow('UPLOAD_TYPE_MISMATCH'); + + expect(mockUploadWithContentType).not.toHaveBeenCalled(); + }); + + it('accepts a text file whose extension the bytes cannot confirm in detail', async () => { + // Leading bytes can tell text from binary, not CSV from plain text; treating + // that as a mismatch would reject every legitimate text upload. + const { constructiveUploadFieldDefinitions } = + await loadUploadResolverModule({ detectedContentType: 'text/plain' }); + const { context } = fakeContext(); + + const result: any = await definitionFor(constructiveUploadFieldDefinitions, 'upload').resolve( + { ...makeFakeUpload('rows.csv'), mimetype: 'text/csv' } as any, + {}, + context, + { uploadPlugin: { tags: {}, type: 'upload', field: FIELD } }, + ); + + expect(result.id).toBe(FILE_ID); + }); }); diff --git a/graphile/graphile-settings/package.json b/graphile/graphile-settings/package.json index 6af2c66fb2..b9cc32a949 100644 --- a/graphile/graphile-settings/package.json +++ b/graphile/graphile-settings/package.json @@ -67,6 +67,7 @@ "graphql": "16.13.0", "inflekt": "^0.8.1", "lru-cache": "^11.2.7", + "mime-bytes": "workspace:^", "pg": "^8.21.0", "pg-query-context": "workspace:^", "pg-sql2": "5.0.1", diff --git a/graphile/graphile-settings/src/upload-resolver.ts b/graphile/graphile-settings/src/upload-resolver.ts index 945b3efc33..104f1459cb 100644 --- a/graphile/graphile-settings/src/upload-resolver.ts +++ b/graphile/graphile-settings/src/upload-resolver.ts @@ -41,6 +41,7 @@ import type { UploadFieldIdentity, UploadPluginInfo, } from 'graphile-upload-plugin'; +import { checkTypeAgreement } from 'mime-bytes'; import { Transform } from 'stream'; import { @@ -228,6 +229,25 @@ async function uploadResolver( readStream: upload.createReadStream(), filename, }); + + // The three claims must agree, and disagreement is rejection rather than + // relabelling: an `.jpg` carrying HTML is served to a browser as an image and + // executed as a script. Compared against `magic.type` — what the bytes say — + // not `contentType`, which the detector may have refined using the very + // extension under suspicion. + const agreement = checkTypeAgreement({ + filename, + declaredMime: upload.mimetype, + detectedMime: detected.magic?.type, + }); + if (!agreement.ok) { + detected.stream.destroy(); + throw new Error( + `UPLOAD_TYPE_MISMATCH: ${agreement.violation.message}. The upload was rejected rather than ` + + 'stored under a type it is not.', + ); + } + const allowed = allowedMimeTypes(tags, typ); if (allowed.length && !allowed.includes(detected.contentType)) { detected.stream.destroy(); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 27061008c5..257ab13c58 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -964,6 +964,9 @@ importers: lru-cache: specifier: ^11.2.7 version: 11.2.7 + mime-bytes: + specifier: workspace:^ + version: link:../../uploads/mime-bytes/dist postgraphile: specifier: 5.1.3 version: 5.1.3(d50eb5de8c32cb565d62d5f80f785107) @@ -1315,6 +1318,9 @@ importers: lru-cache: specifier: ^11.2.7 version: 11.2.7 + mime-bytes: + specifier: workspace:^ + version: link:../../uploads/mime-bytes/dist pg: specifier: ^8.21.0 version: 8.21.0 diff --git a/uploads/mime-bytes/__tests__/type-agreement.test.ts b/uploads/mime-bytes/__tests__/type-agreement.test.ts new file mode 100644 index 0000000000..749a4b27bc --- /dev/null +++ b/uploads/mime-bytes/__tests__/type-agreement.test.ts @@ -0,0 +1,118 @@ +import { createReadStream } from 'fs'; +import path from 'path'; + +import { FileTypeDetector } from '../src/file-type-detector'; +import { checkTypeAgreement, mimeTypeForFilename, mimeTypesAgree } from '../src/type-agreement'; + +describe('mimeTypeForFilename', () => { + test('reads the extension, not the name', () => { + expect(mimeTypeForFilename('holiday.photo.jpg')).toBe('image/jpeg'); + expect(mimeTypeForFilename('report.pdf')).toBe('application/pdf'); + }); + + test('claims nothing without an extension', () => { + expect(mimeTypeForFilename('LICENSE')).toBeNull(); + expect(mimeTypeForFilename('')).toBeNull(); + expect(mimeTypeForFilename(null)).toBeNull(); + }); +}); + +describe('mimeTypesAgree', () => { + test('silence agrees with everything', () => { + expect(mimeTypesAgree(null, 'image/png')).toBe(true); + expect(mimeTypesAgree('application/octet-stream', 'image/png')).toBe(true); + }); + + test('aliases of one type are that type', () => { + expect(mimeTypesAgree('image/jpg', 'image/jpeg')).toBe(true); + expect(mimeTypesAgree('text/xml', 'application/xml')).toBe(true); + }); + + test('text is one family, because leading bytes cannot split it', () => { + expect(mimeTypesAgree('text/csv', 'text/plain')).toBe(true); + expect(mimeTypesAgree('application/json', 'text/plain')).toBe(true); + expect(mimeTypesAgree('image/svg+xml', 'text/plain')).toBe(true); + }); + + test('a container agrees with what it contains', () => { + expect( + mimeTypesAgree( + 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', + 'application/zip', + ), + ).toBe(true); + expect(mimeTypesAgree('audio/mp4', 'video/mp4')).toBe(true); + }); + + test('different kinds of file disagree', () => { + expect(mimeTypesAgree('image/jpeg', 'text/html')).toBe(false); + expect(mimeTypesAgree('image/jpeg', 'image/png')).toBe(false); + expect(mimeTypesAgree('application/pdf', 'application/zip')).toBe(false); + }); +}); + +describe('checkTypeAgreement', () => { + test('accepts a file that is what it says it is', () => { + expect( + checkTypeAgreement({ + filename: 'avatar.png', + declaredMime: 'image/png', + detectedMime: 'image/png', + }), + ).toEqual({ ok: true }); + }); + + test('rejects an extension that lies about the bytes', () => { + const result = checkTypeAgreement({ + filename: 'avatar.jpg', + declaredMime: 'image/jpeg', + detectedMime: 'text/html', + }); + expect(result.ok).toBe(false); + expect(result.violation?.code).toBe('EXTENSION_BYTES_MISMATCH'); + expect(result.violation?.message).toContain('image/jpeg'); + expect(result.violation?.message).toContain('text/html'); + }); + + test('rejects a declared type that lies about the bytes', () => { + const result = checkTypeAgreement({ + filename: 'notes', + declaredMime: 'image/png', + detectedMime: 'application/pdf', + }); + expect(result.ok).toBe(false); + expect(result.violation?.code).toBe('DECLARED_BYTES_MISMATCH'); + }); + + test('rejects a declared type that contradicts the extension, with no bytes seen', () => { + const result = checkTypeAgreement({ + filename: 'invoice.pdf', + declaredMime: 'image/png', + }); + expect(result.ok).toBe(false); + expect(result.violation?.code).toBe('DECLARED_EXTENSION_MISMATCH'); + }); + + test('passes an upload whose bytes have not been seen and whose claims agree', () => { + expect(checkTypeAgreement({ filename: 'invoice.pdf', declaredMime: 'application/pdf' }).ok).toBe(true); + }); + + test('does not reject an extension it has never heard of', () => { + expect(checkTypeAgreement({ filename: 'model.myformat', detectedMime: 'application/pdf' }).ok).toBe(true); + }); + + test('rejects the html-as-jpg fixture, detecting from its actual bytes', async () => { + const fixture = path.join(__dirname, '../../../__fixtures__/malicious/html-as-jpg.jpg'); + const detector = new FileTypeDetector(); + const detected = await detector.detectWithFallback(createReadStream(fixture), 'html-as-jpg.jpg'); + + const result = checkTypeAgreement({ + filename: 'html-as-jpg.jpg', + declaredMime: 'image/jpeg', + detectedMime: detected?.mimeType, + }); + + expect(result.ok).toBe(false); + expect(result.violation?.code).toBe('EXTENSION_BYTES_MISMATCH'); + }); +}); diff --git a/uploads/mime-bytes/src/index.ts b/uploads/mime-bytes/src/index.ts index f43fb6de53..a0d16d982e 100644 --- a/uploads/mime-bytes/src/index.ts +++ b/uploads/mime-bytes/src/index.ts @@ -30,6 +30,15 @@ export { PeekPromise, PeekStreamOptions} from './peak'; +// Export declared-type / extension / magic-byte agreement checking +export { + checkTypeAgreement, + mimeTypeForFilename, + mimeTypesAgree, + TypeAgreementInput, + TypeAgreementResult, + TypeAgreementViolation} from './type-agreement'; + // Export utility functions export * from './utils/extensions'; export * from './utils/magic-bytes'; diff --git a/uploads/mime-bytes/src/type-agreement.ts b/uploads/mime-bytes/src/type-agreement.ts new file mode 100644 index 0000000000..edbfedaa4b --- /dev/null +++ b/uploads/mime-bytes/src/type-agreement.ts @@ -0,0 +1,202 @@ +/** + * Agreement between the three things that claim to say what a file is. + * + * An upload arrives with up to three independent statements about its type: + * + * * the **declared** MIME type — what the client says it is; + * * the **extension** on the filename — what the client says it is again, in a + * form the operating system and the browser act on; + * * the **magic bytes** — what it actually is. + * + * Only the third is evidence. The other two are the attack surface: a browser + * that fetches `avatar.jpg` and receives HTML will happily run the script in it, + * which is why `html-as-jpg.jpg` is a fixture in this repo rather than a + * curiosity. So the rule is agreement, not precedence: when two statements + * describe different kinds of file, the upload is rejected instead of being + * silently relabelled. + * + * Deliberately conservative about what counts as disagreement, because a false + * rejection is a broken upload: + * + * * a statement that is absent or `application/octet-stream` says nothing, and + * cannot contradict anything; + * * text formats are indistinguishable by leading bytes — a `.csv`, a `.json` + * and an `.svg` are all "some text" — so text is compared as one family; + * * container formats really are the thing they contain: a `.docx` *is* a ZIP, + * an `.m4a` *is* an MP4 box stream. Those pairs are declared equivalent. + */ + +import { getContentTypeByExtension } from './file-types-registry'; +import { getExtension } from './utils/extensions'; +import { normalizeMimeType, resolveMimeAlias } from './utils/mime-types'; + +/** Says nothing about the content, so it can never contradict anything. */ +const UNKNOWN_MIME = 'application/octet-stream'; + +/** + * Types whose bytes are text, and are therefore mutually indistinguishable to a + * magic-byte sniff: it can tell text from binary, not JSON from CSV. + */ +const TEXT_LIKE = [ + 'text/', + 'application/json', + 'application/ld+json', + 'application/xml', + 'application/xhtml+xml', + 'application/javascript', + 'application/ecmascript', + 'application/typescript', + 'application/x-httpd-php', + 'application/x-sh', + 'application/x-csh', + 'application/x-python', + 'application/x-ruby', + 'application/x-perl', + 'application/x-yaml', + 'application/yaml', + 'application/toml', + 'application/sql', + 'image/svg+xml', +]; + +/** + * Formats that are physically another format: their leading bytes are the + * container's, so a sniff naming the container is not a contradiction. + * + * Keyed by the container the bytes look like; the values are the specific types + * that legitimately sit inside it. + */ +const CONTAINER_FAMILIES: Record = { + 'application/zip': [ + 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', + 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', + 'application/vnd.openxmlformats-officedocument.presentationml.presentation', + 'application/vnd.oasis.opendocument.text', + 'application/vnd.oasis.opendocument.spreadsheet', + 'application/vnd.oasis.opendocument.presentation', + 'application/epub+zip', + 'application/java-archive', + 'application/vnd.android.package-archive', + 'application/x-xpinstall', + ], + 'video/mp4': ['audio/mp4', 'audio/x-m4a', 'video/quicktime', 'video/x-m4v'], + 'audio/mp4': ['video/mp4', 'audio/x-m4a', 'video/x-m4v'], + 'video/quicktime': ['video/mp4'], + 'application/x-riff': ['audio/wav', 'audio/x-wav', 'video/avi', 'video/x-msvideo', 'image/webp'], + 'application/gzip': ['application/tar', 'application/x-gtar'], +}; + +function isTextLike(mime: string): boolean { + return TEXT_LIKE.some((prefix) => mime === prefix || mime.startsWith(prefix)); +} + +/** A statement that carries no information about the content. */ +function isSilent(mime: string | null | undefined): boolean { + return !mime || mime === UNKNOWN_MIME; +} + +function canonical(mime: string | null | undefined): string | null { + if (!mime) return null; + const normalized = resolveMimeAlias(normalizeMimeType(mime)); + return normalized || null; +} + +function inSameContainerFamily(a: string, b: string): boolean { + return (CONTAINER_FAMILIES[a]?.includes(b) ?? false) || (CONTAINER_FAMILIES[b]?.includes(a) ?? false); +} + +/** + * Whether two type statements describe the same kind of file. + * + * Silence agrees with everything; text agrees with text; a container agrees with + * what it contains. Everything else must match exactly. + */ +export function mimeTypesAgree(a: string | null | undefined, b: string | null | undefined): boolean { + const left = canonical(a); + const right = canonical(b); + + if (isSilent(left) || isSilent(right)) return true; + if (left === right) return true; + if (isTextLike(left!) && isTextLike(right!)) return true; + return inSameContainerFamily(left!, right!); +} + +/** The MIME type a filename's extension claims, or null when it claims nothing. */ +export function mimeTypeForFilename(filename: string | null | undefined): string | null { + if (!filename) return null; + const extension = getExtension(filename); + if (!extension) return null; + return canonical(getContentTypeByExtension(extension)); +} + +export interface TypeAgreementInput { + /** The uploaded filename, if any. Its extension is one of the claims. */ + filename?: string | null; + /** The MIME type the client declared for the upload. */ + declaredMime?: string | null; + /** + * The MIME type detected from the file's leading bytes. Omitted when the bytes + * have not been seen — a presigned upload before its confirm read — in which + * case only the two client-supplied claims are compared with each other. + */ + detectedMime?: string | null; +} + +export type TypeAgreementViolation = { + code: 'EXTENSION_BYTES_MISMATCH' | 'DECLARED_BYTES_MISMATCH' | 'DECLARED_EXTENSION_MISMATCH'; + message: string; + filename?: string | null; + extensionMime?: string | null; + declaredMime?: string | null; + detectedMime?: string | null; +}; + +export type TypeAgreementResult = + | { ok: true; violation?: undefined } + | { ok: false; violation: TypeAgreementViolation }; + +/** + * Check the declared type, the filename's extension and the detected bytes + * against each other. + * + * Returns the first disagreement found, bytes-first: when bytes are available + * they are the evidence, so "the extension lies about the bytes" is the more + * useful thing to report than "the two client claims differ". + */ +export function checkTypeAgreement(input: TypeAgreementInput): TypeAgreementResult { + const { filename } = input; + const declaredMime = canonical(input.declaredMime); + const detectedMime = canonical(input.detectedMime); + const extensionMime = mimeTypeForFilename(filename); + + const violation = ( + code: TypeAgreementViolation['code'], + message: string, + ): TypeAgreementResult => ({ + ok: false, + violation: { code, message, filename, extensionMime, declaredMime, detectedMime }, + }); + + if (!mimeTypesAgree(extensionMime, detectedMime)) { + return violation( + 'EXTENSION_BYTES_MISMATCH', + `file "${filename}" is named as ${extensionMime} but its bytes are ${detectedMime}`, + ); + } + + if (!mimeTypesAgree(declaredMime, detectedMime)) { + return violation( + 'DECLARED_BYTES_MISMATCH', + `upload declares ${declaredMime} but its bytes are ${detectedMime}`, + ); + } + + if (!mimeTypesAgree(declaredMime, extensionMime)) { + return violation( + 'DECLARED_EXTENSION_MISMATCH', + `upload declares ${declaredMime} but is named "${filename}", which is ${extensionMime}`, + ); + } + + return { ok: true }; +}