From b6f12f7e2c61d60367afdb8e0b63748838b19afb Mon Sep 17 00:00:00 2001 From: Timothy Date: Sun, 12 Jul 2026 00:15:56 +0200 Subject: [PATCH] security(#283): clamp served artwork MIME type to the image allow-list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cold-review LOW (defense-in-depth): the serve path derived the Content-Type from the stored file via Winista but only defaulted application/octet-stream on a NULL sniff. A cache file whose bytes are HTML — a legacy entry poisoned before the upload-sniff landed, or a hypothetical image/script polyglot — could still be sniffed as text/html and served renderable (nosniff does not stop an explicitly declared text/html). Clamp the sniffed type to ImageContentTypes.IsAccepted, serving application/octet-stream for anything else, so the serve path can never emit a renderable non-image type regardless of what bytes are on disk. Refs #283 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Images/Queries/GetCachedImagePathHandler.cs | 10 +++++++--- docs/decisions.md | 8 +++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/ErsatzTV.Application/Images/Queries/GetCachedImagePathHandler.cs b/ErsatzTV.Application/Images/Queries/GetCachedImagePathHandler.cs index d583e11bd..380ac3b4f 100644 --- a/ErsatzTV.Application/Images/Queries/GetCachedImagePathHandler.cs +++ b/ErsatzTV.Application/Images/Queries/GetCachedImagePathHandler.cs @@ -1,6 +1,7 @@ using CliWrap; using ErsatzTV.Core; using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Images; using ErsatzTV.Core.Interfaces.FFmpeg; using ErsatzTV.Core.Interfaces.Images; using ErsatzTV.Core.Interfaces.Repositories; @@ -96,9 +97,12 @@ public class else { // Always derive the type from the stored file — never from a client-supplied value - // (issue #283 — the old ?contentType= reflection was the stored-XSS sink). Fall back - // to a non-renderable default if the sniffer can't identify the bytes. - mimeType = MimeTypes.GetMimeTypeFromFile(cachePath)?.Name ?? "application/octet-stream"; + // (issue #283 — the old ?contentType= reflection was the stored-XSS sink). Clamp the + // sniffed type to the image allow-list so a file whose bytes are not an accepted image + // (a legacy cache entry poisoned before the upload sniff landed, or a hypothetical + // polyglot) is served as a non-renderable download, never as HTML/script. + string sniffed = MimeTypes.GetMimeTypeFromFile(cachePath)?.Name; + mimeType = ImageContentTypes.IsAccepted(sniffed) ? sniffed : "application/octet-stream"; } return new CachedImagePathViewModel(cachePath, mimeType); diff --git a/docs/decisions.md b/docs/decisions.md index 5d7246e48..42292746c 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -959,9 +959,11 @@ honors regardless of `nosniff`. The trust was the bug; the fix removes it at bot - **Serve sniffs the stored file; the `?contentType=` reflection is gone.** `GetCachedImagePath` no longer carries a `ContentType`, and `GetImage` (`/iptv/logos`) / `GetWatermark` (`/artwork/watermarks`) dropped their `[FromQuery] contentType` binding. `GetCachedImagePathHandler` always derives the MIME type from the - file (`MimeTypes.GetMimeTypeFromFile`), defaulting to `application/octet-stream` if unidentifiable. The - removal is **structural** — there is no longer any request path that lets a client choose the served - `Content-Type`. `ArtworkContentTypeModel.UrlWithContentType` now returns the bare path, and the SPA + file (`MimeTypes.GetMimeTypeFromFile`) and **clamps it to the image allow-list** (`ImageContentTypes.IsAccepted`), + serving `application/octet-stream` for anything else — so a file whose bytes are not an accepted image (a + legacy cache entry poisoned before the upload sniff landed, or a hypothetical polyglot) is a non-renderable + download, never HTML/script. The removal is **structural** — there is no longer any request path that lets a + client choose the served `Content-Type`. `ArtworkContentTypeModel.UrlWithContentType` now returns the bare path, and the SPA watermark/logo previews no longer append the query. - **Defense-in-depth on the persisted JSON DTOs.** The `{path, contentType}` bodies (channel logo, watermark) run their content type through `ArtworkContentTypeModel.Sanitized()`, which blanks anything outside the