[img] hardening

apps/api/src/routes/avatar.ts

@@ -482,7 +482,7 @@ export const avatarRoutes = new Hono<App>({ strict: false })
 				'`avatar-item/<date>/<id>-thumb.png` and `avatar-item/<date>/<id>-design.png`; those ' +
 				'keys are the `ThumbnailImageFilename` / `DesignFilename` on the row. Each file is ' +
 				'identified by its bytes — the client posts them under `.bin` names — and must be a ' +
-				'PNG, JPEG, GIF, WebP or BMP; the key’s extension and the stored content-type come ' +
+				'PNG or JPEG; the key’s extension and the stored content-type come ' +
 				'from that signature, and anything else is refused with a 400 before either file is ' +
 				'written.\n\n' +
 				'`Price` is capped at 1000 tokens (`MAX_PLAYER_ITEM_PRICE`); one above it, or below ' +

apps/api/src/routes/images.ts

@@ -220,10 +220,9 @@ export const imageRoutes = new Hono<App>({ strict: false })
 				'`savedImageType` of 4 (profile thumbnail) additionally becomes the account’s ' +
 				'avatar, persisted on the account row.\n\n' +
 				'The file is identified by its bytes, not by the part’s filename or content-type: ' +
-				'it must be a PNG, JPEG, GIF, WebP or BMP, and the stored key’s extension and ' +
+				'it must be a PNG or JPEG, and the stored key’s extension and ' +
 				'content-type come from that signature. Anything else is refused with a 400 — the ' +
-				'`img` worker serves the object back under its stored type, so an HTML body under ' +
-				'an image name would otherwise be a page on the image origin.',
+				'`img` worker serves only those two formats, so it would never display.',
 			security: AUTHED,
 			requestBody: form(UploadImageRequest, 'The image file plus its metadata'),
 			responses: {

apps/api/src/test/integration/api.test.ts

@@ -5017,14 +5017,10 @@ describe('custom avatar items', () => {
 
 		// Both uploads landed in the image bucket under those keys.
 		const thumb = await env.IMAGES.get(body.Value.ThumbnailImageFilename as string)
-		expect(new Uint8Array((await thumb!.arrayBuffer()) as ArrayBuffer)).toEqual(
-			new Uint8Array([1, 2, 3])
-		)
+		expect(new Uint8Array((await thumb!.arrayBuffer()) as ArrayBuffer)).toEqual(PNG_BYTES)
 		expect(thumb!.httpMetadata?.contentType).toBe('image/png')
 		const design = await env.IMAGES.get(body.Value.DesignFilename as string)
-		expect(new Uint8Array((await design!.arrayBuffer()) as ArrayBuffer)).toEqual(
-			new Uint8Array([4, 5, 6])
-		)
+		expect(new Uint8Array((await design!.arrayBuffer()) as ArrayBuffer)).toEqual(PNG_BYTES)
 
 		// The row is the record as JSON, with the queried fields generated off it.
 		const row = await env.DB.prepare(
@@ -5384,14 +5380,14 @@ describe('custom avatar items', () => {
 		expect(badThumbnail.status).toBe(400)
 		expect(await badThumbnail.json()).toMatchObject({
 			Success: false,
-			Error: 'thumbnailImage is not a PNG, JPEG, GIF, WebP or BMP image',
+			Error: 'thumbnailImage is not a PNG or JPEG image',
 		})
 
 		const badDesign = await upload(PNG_BYTES, html)
 		expect(badDesign.status).toBe(400)
 		expect(await badDesign.json()).toMatchObject({
 			Success: false,
-			Error: 'design is not a PNG, JPEG, GIF, WebP or BMP image',
+			Error: 'design is not a PNG or JPEG image',
 		})
 
 		const row = await env.DB.prepare(
@@ -6974,7 +6970,7 @@ describe('images', () => {
 			body: fd,
 		})
 		expect(res.status).toBe(400)
-		expect(await res.json()).toEqual({ error: 'image is not a PNG, JPEG, GIF, WebP or BMP image' })
+		expect(await res.json()).toEqual({ error: 'image is not a PNG or JPEG image' })
 		expect((await env.IMAGES.list()).objects).toHaveLength(objectsBefore)
 	})
 

apps/img/src/img.app.ts

@@ -54,6 +54,18 @@ function isPng(bytes: Uint8Array): boolean {
 	return PNG_SIGNATURE.every((b, i) => bytes[i] === b)
 }
 
+/** The only content-types served from R2. */
+const SERVED_CONTENT_TYPES = new Set(['image/png', 'image/jpeg'])
+
+/**
+ * The stored content-type, bare and lower-cased, when it is one this worker serves —
+ * otherwise `undefined`.
+ */
+function servedContentType(stored: string | undefined): string | undefined {
+	const type = stored?.split(';')[0]?.trim().toLowerCase()
+	return type && SERVED_CONTENT_TYPES.has(type) ? type : undefined
+}
+
 /** A requested transform, from `?width=`/`?height=`/`?cropSquare=1`. At least one applies. */
 interface Transform {
 	width?: number
@@ -529,21 +541,36 @@ app.get(
 		// bytes that are not the range asked for — ask R2 for the range only when we are
 		// going to hand its bytes straight back. R2 parses the header itself; see
 		// writeContentRange() below for why it is never answered with a bare 200.
-		const range = needsBody(transform, signing) ? undefined : c.req.raw.headers
+		const buffered = needsBody(transform, signing)
+		const range = buffered ? undefined : c.req.raw.headers
 		const object = await bucket.get(objectKey, {
 			...(ifNoneMatch ? { onlyIf: { etagDoesNotMatch: ifNoneMatch } } : {}),
 			...(range ? { range } : {}),
 		})
-		if (!object) {
-			// Missing from both static and R2 → serve the bundled DefaultProfileImage.jpg
-			// static asset so clients still get a valid image instead of a 404. Honour
-			// `?sig=p1` the same way so the fallback is signed like any other image.
+
+		// Missing from both static and R2, or not a PNG/JPEG → serve the bundled
+		// DefaultProfileImage.jpg static asset so clients still get a valid image instead
+		// of a 404. Honour `?sig=p1` the same way so the fallback is signed like any other.
+		const serveFallback = async () => {
 			const asset = await c.env.ASSETS.fetch(new URL(FALLBACK_ASSET_PATH, c.req.url))
 			return serveStaticAsset(c.env, asset, transform, signing)
 		}
+		if (!object) return serveFallback()
+
+		// Only a PNG or JPEG is ever served, judged by the object's stored content-type.
+		// Uploads store the type their bytes name (`sniffImage` in every writer), so the
+		// stored type is trusted rather than re-reading the bytes here; anything else —
+		// an older upload stored under the uploader's `text/html` claim, say — is treated
+		// as missing.
+		const storedType = servedContentType(object.httpMetadata?.contentType)
+		if (!storedType) {
+			if ('body' in object) await object.body.cancel()
+			return serveFallback()
+		}
 
 		const headers = new Headers()
 		object.writeHttpMetadata(headers)
+		headers.set('content-type', storedType)
 		headers.set('etag', object.httpEtag)
 		headers.set('cache-control', CACHE_CONTROL)
 
@@ -554,7 +581,7 @@ app.get(
 		// ever rides a response that actually carries bytes.
 		applyStubSignature(headers, signing)
 
-		if (needsBody(transform, signing)) {
+		if (buffered) {
 			const bytes = await object.arrayBuffer()
 			return finalizeImage(c.env, bytes, headers, transform, signing)
 		}

apps/img/src/test/integration/api.test.ts

@@ -107,6 +107,52 @@ describe('img endpoints', () => {
 		expect(notModified.headers.get('x-content-type-options')).toBe('nosniff')
 	})
 
+	// Only an object stored as PNG or JPEG is ever served. Anything else gets the
+	// fallback, as though the key were missing.
+	it('serves the fallback, not the body, for an object not stored as PNG or JPEG', async () => {
+		const fallback = new Uint8Array(
+			await (await SELF.fetch(`${ORIGIN}/does-not-exist-anywhere.jpg`)).arrayBuffer()
+		)
+		const junk = [
+			{ key: 'gate/page.jpg', body: '<html><script>alert(1)</script></html>', type: 'text/html' },
+			{ key: 'gate/octet.jpg', body: IMAGE_BYTES, type: 'application/octet-stream' },
+			{ key: 'gate/anim.gif', body: 'GIF89a\x01\x00\x01\x00', type: 'image/gif' },
+		]
+		for (const { key, body, type } of junk) {
+			await env.IMAGES.put(key, body, { httpMetadata: { contentType: type } })
+			for (const query of ['', '?width=128', '?sig=p1']) {
+				const res = await SELF.fetch(`${ORIGIN}/${key}${query}`)
+				expect(res.status, key + query).toBe(200)
+				expect(res.headers.get('content-type'), key + query).toBe('image/jpeg')
+				if (query === '') expect(new Uint8Array(await res.arrayBuffer()), key).toEqual(fallback)
+				else await res.arrayBuffer()
+			}
+		}
+
+		// A range into a non-image is refused too, not just the whole-object read.
+		const ranged = await SELF.fetch(`${ORIGIN}/gate/page.jpg`, { headers: { range: 'bytes=6-20' } })
+		expect(ranged.status).toBe(200)
+		expect(new Uint8Array(await ranged.arrayBuffer())).toEqual(fallback)
+	})
+
+	// The served type is the stored one, normalised — never a parameter riding on it.
+	it('serves a stored image type bare, and a range of it', async () => {
+		await env.IMAGES.put('gate/params.jpg', IMAGE_BYTES, {
+			httpMetadata: { contentType: 'IMAGE/JPEG; charset=utf-8' },
+		})
+		const res = await SELF.fetch(`${ORIGIN}/gate/params.jpg`)
+		expect(res.status).toBe(200)
+		expect(res.headers.get('content-type')).toBe('image/jpeg')
+		expect(new Uint8Array(await res.arrayBuffer())).toEqual(IMAGE_BYTES)
+
+		const ranged = await SELF.fetch(`${ORIGIN}/gate/params.jpg`, {
+			headers: { range: 'bytes=4-7' },
+		})
+		expect(ranged.status).toBe(206)
+		expect(ranged.headers.get('content-type')).toBe('image/jpeg')
+		expect(new Uint8Array(await ranged.arrayBuffer())).toEqual(IMAGE_BYTES.slice(4, 8))
+	})
+
 	it('streams an image stored in R2 with its content type', async () => {
 		const res = await SELF.fetch(`${ORIGIN}/${R2_KEY}`)
 		expect(res.status).toBe(200)

apps/storage/src/test/integration/api.test.ts

@@ -189,7 +189,7 @@ it('POST /upload refuses an Image (FileType 3) whose bytes are not an image', as
 		body: form,
 	})
 	expect(res.status).toBe(400)
-	expect(await res.json()).toEqual({ error: 'file is not a PNG, JPEG, GIF, WebP or BMP image' })
+	expect(await res.json()).toEqual({ error: 'file is not a PNG or JPEG image' })
 })
 
 it('POST /upload stores an Image (FileType 3) under the type its bytes say, not the claim', async () => {