diff --git a/ErsatzTV.Application/Artworks/ArtworkContentTypeModel.cs b/ErsatzTV.Application/Artworks/ArtworkContentTypeModel.cs index f2d75c62a..393af4d46 100644 --- a/ErsatzTV.Application/Artworks/ArtworkContentTypeModel.cs +++ b/ErsatzTV.Application/Artworks/ArtworkContentTypeModel.cs @@ -1,5 +1,5 @@ -using System.Net; using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Images; namespace ErsatzTV.Application.Artworks; @@ -11,7 +11,14 @@ public record ArtworkContentTypeModel(string Path, string ContentType) public bool HasContentType => !string.IsNullOrWhiteSpace(ContentType); - public string UrlWithContentType => string.IsNullOrWhiteSpace(ContentType) - ? Path - : $"{Path}?contentType={WebUtility.UrlEncode(ContentType)}"; + // The artwork serve routes now sniff the content type from the stored file and no longer honor a + // client-supplied ?contentType= (issue #283 — that reflection was the stored-XSS sink), so the + // directly-usable URL is just the path. + public string UrlWithContentType => Path; + + // Defense-in-depth: never persist a content type outside the image allow-list, so a value that + // slipped in via the {path, contentType} JSON DTOs can't later be reflected anywhere. The serve + // path derives the type from the file regardless; this only keeps stored metadata honest. + public ArtworkContentTypeModel Sanitized() => + ImageContentTypes.IsAccepted(ContentType) ? this : this with { ContentType = string.Empty }; } diff --git a/ErsatzTV.Application/Artworks/Commands/UploadArtwork.cs b/ErsatzTV.Application/Artworks/Commands/UploadArtwork.cs index e27c60b37..84da5d211 100644 --- a/ErsatzTV.Application/Artworks/Commands/UploadArtwork.cs +++ b/ErsatzTV.Application/Artworks/Commands/UploadArtwork.cs @@ -9,5 +9,5 @@ namespace ErsatzTV.Application.Artworks; /// landing it in the same on-disk cache the Blazor UI uses (via IImageCache), /// so the returned path is equivalent to a Blazor-uploaded image. /// -public record UploadArtwork(Stream Stream, string ContentType, ArtworkKind ArtworkKind) +public record UploadArtwork(Stream Stream, ArtworkKind ArtworkKind) : IRequest>; diff --git a/ErsatzTV.Application/Artworks/Commands/UploadArtworkHandler.cs b/ErsatzTV.Application/Artworks/Commands/UploadArtworkHandler.cs index d8623baea..c2ab153d9 100644 --- a/ErsatzTV.Application/Artworks/Commands/UploadArtworkHandler.cs +++ b/ErsatzTV.Application/Artworks/Commands/UploadArtworkHandler.cs @@ -1,22 +1,13 @@ using ErsatzTV.Core; using ErsatzTV.Core.Api.Artwork; using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Images; using ErsatzTV.Core.Interfaces.Images; namespace ErsatzTV.Application.Artworks; public class UploadArtworkHandler : IRequestHandler> { - // png/jpeg/gif/webp are all decoded by SkiaSharp and read by FFmpeg, matching the - // formats the Blazor logo/watermark upload already accepts. Format expansion is ersatztv#66. - private static readonly System.Collections.Generic.HashSet AcceptedContentTypes = new(StringComparer.OrdinalIgnoreCase) - { - "image/png", - "image/jpeg", - "image/gif", - "image/webp" - }; - private readonly IImageCache _imageCache; public UploadArtworkHandler(IImageCache imageCache) => _imageCache = imageCache; @@ -25,15 +16,31 @@ public class UploadArtworkHandler : IRequestHandler payload be stored and later served + // as HTML). A payload that isn't a supported raster image is rejected here. + Option maybeContentType = ImageContentTypes.DetectContentType(bytes); + if (maybeContentType.IsNone) + { + return BaseError.New( + $"Uploaded file is not a supported image; supported types are: {string.Join(", ", ImageContentTypes.Accepted)}"); + } + + string contentType = maybeContentType.IfNone(string.Empty); + + using var toCache = new MemoryStream(bytes, writable: false); Either maybeFileName = await _imageCache.SaveArtworkToCache( - request.Stream, + toCache, request.ArtworkKind); return maybeFileName.Map(fileName => new ArtworkUploadResponseModel( diff --git a/ErsatzTV.Application/Images/Queries/GetCachedImagePath.cs b/ErsatzTV.Application/Images/Queries/GetCachedImagePath.cs index 1fa0d9774..d44f8e7c8 100644 --- a/ErsatzTV.Application/Images/Queries/GetCachedImagePath.cs +++ b/ErsatzTV.Application/Images/Queries/GetCachedImagePath.cs @@ -3,6 +3,6 @@ using ErsatzTV.Core.Domain; namespace ErsatzTV.Application.Images; -public record GetCachedImagePath(string FileName, ArtworkKind ArtworkKind, string ContentType, int? MaxHeight = null) +public record GetCachedImagePath(string FileName, ArtworkKind ArtworkKind, int? MaxHeight = null) : IRequest< Either>; diff --git a/ErsatzTV.Application/Images/Queries/GetCachedImagePathHandler.cs b/ErsatzTV.Application/Images/Queries/GetCachedImagePathHandler.cs index e6575f45b..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; @@ -95,9 +96,13 @@ public class } else { - mimeType = !string.IsNullOrWhiteSpace(request.ContentType) - ? request.ContentType - : MimeTypes.GetMimeTypeFromFile(cachePath).Name; + // 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). 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/ErsatzTV.Core.Tests/Images/ImageContentTypesTests.cs b/ErsatzTV.Core.Tests/Images/ImageContentTypesTests.cs new file mode 100644 index 000000000..902b908db --- /dev/null +++ b/ErsatzTV.Core.Tests/Images/ImageContentTypesTests.cs @@ -0,0 +1,60 @@ +using System.Text; +using ErsatzTV.Core.Images; +using NUnit.Framework; +using Shouldly; +using SkiaSharp; + +namespace ErsatzTV.Core.Tests.Images; + +[TestFixture] +public class ImageContentTypesTests +{ + [TestCase(SKEncodedImageFormat.Png, "image/png")] + [TestCase(SKEncodedImageFormat.Jpeg, "image/jpeg")] + [TestCase(SKEncodedImageFormat.Webp, "image/webp")] + public void DetectContentType_Should_Sniff_Real_Image_Bytes(SKEncodedImageFormat format, string expected) + { + byte[] bytes = EncodeTinyImage(format); + + Option detected = ImageContentTypes.DetectContentType(bytes); + + detected.IfNone(string.Empty).ShouldBe(expected); + } + + [Test] + public void DetectContentType_Should_Reject_Non_Image_Payload() + { + // The stored-XSS payload from issue #283 — script bytes that a spoofed content type would + // otherwise have let through. + byte[] html = Encoding.UTF8.GetBytes(""); + + ImageContentTypes.DetectContentType(html).IsNone.ShouldBeTrue(); + } + + [Test] + public void DetectContentType_Should_Return_None_For_Empty_Or_Null() + { + ImageContentTypes.DetectContentType([]).IsNone.ShouldBeTrue(); + ImageContentTypes.DetectContentType(null).IsNone.ShouldBeTrue(); + } + + [TestCase("image/png", true)] + [TestCase("image/jpeg", true)] + [TestCase("image/gif", true)] + [TestCase("image/webp", true)] + [TestCase("IMAGE/PNG", true)] + [TestCase("text/html", false)] + [TestCase("image/svg+xml", false)] + [TestCase("", false)] + [TestCase(null, false)] + public void IsAccepted_Should_Allow_Only_Raster_Image_Types(string contentType, bool expected) => + ImageContentTypes.IsAccepted(contentType).ShouldBe(expected); + + private static byte[] EncodeTinyImage(SKEncodedImageFormat format) + { + using var bitmap = new SKBitmap(2, 2); + using SKImage image = SKImage.FromBitmap(bitmap); + using SKData data = image.Encode(format, 100); + return data.ToArray(); + } +} diff --git a/ErsatzTV.Core/Images/ImageContentTypes.cs b/ErsatzTV.Core/Images/ImageContentTypes.cs new file mode 100644 index 000000000..26acd47c2 --- /dev/null +++ b/ErsatzTV.Core/Images/ImageContentTypes.cs @@ -0,0 +1,64 @@ +using SkiaSharp; + +namespace ErsatzTV.Core.Images; + +/// +/// The single source of truth for which image content types the artwork surfaces accept, plus a +/// byte-sniffing detector used to derive the true content type from an upload's actual contents +/// rather than trusting the client-declared value (issue #283 — stored XSS via a spoofed +/// content type). png/jpeg/gif/webp are the formats SkiaSharp decodes and FFmpeg reads, matching +/// what the artwork upload has always accepted; format expansion is tracked by ersatztv#66. +/// +public static class ImageContentTypes +{ + private static readonly IReadOnlyDictionary FormatContentTypes = + new Dictionary + { + [SKEncodedImageFormat.Png] = "image/png", + [SKEncodedImageFormat.Jpeg] = "image/jpeg", + [SKEncodedImageFormat.Gif] = "image/gif", + [SKEncodedImageFormat.Webp] = "image/webp" + }; + + public static readonly IReadOnlySet Accepted = + new System.Collections.Generic.HashSet(FormatContentTypes.Values, StringComparer.OrdinalIgnoreCase); + + /// + /// Sniffs the true image content type from using SkiaSharp's codec + /// detection. Only the image header is parsed to identify the format — pixels are never + /// decoded here, so a crafted decompression-bomb image cannot be triggered by this call. + /// Returns None for anything that is not one of the accepted raster formats (including + /// non-image payloads such as HTML/script uploaded under a spoofed content type). + /// + public static Option DetectContentType(byte[] bytes) + { + if (bytes is null || bytes.Length == 0) + { + return Option.None; + } + + try + { + using SKData data = SKData.CreateCopy(bytes); + using SKCodec codec = SKCodec.Create(data); + if (codec is null) + { + return Option.None; + } + + return FormatContentTypes.TryGetValue(codec.EncodedFormat, out string contentType) + ? contentType + : Option.None; + } + catch (Exception) + { + return Option.None; + } + } + + /// + /// True when is one of the accepted image content types. + /// + public static bool IsAccepted(string contentType) => + !string.IsNullOrWhiteSpace(contentType) && Accepted.Contains(contentType.Trim()); +} diff --git a/ErsatzTV.Tests/Application/Artworks/ArtworkContentTypeModelTests.cs b/ErsatzTV.Tests/Application/Artworks/ArtworkContentTypeModelTests.cs new file mode 100644 index 000000000..2c61d764e --- /dev/null +++ b/ErsatzTV.Tests/Application/Artworks/ArtworkContentTypeModelTests.cs @@ -0,0 +1,39 @@ +using ErsatzTV.Application.Artworks; +using NUnit.Framework; +using Shouldly; + +namespace ErsatzTV.Tests.Application.Artworks; + +[TestFixture] +public class ArtworkContentTypeModelTests +{ + [Test] + public void Sanitized_Should_Keep_An_Allow_Listed_Content_Type() + { + var model = new ArtworkContentTypeModel("iptv/logos/abc", "image/png"); + + model.Sanitized().ShouldBe(model); + } + + [TestCase("text/html")] + [TestCase("application/javascript")] + [TestCase("image/svg+xml")] + [TestCase("")] + public void Sanitized_Should_Blank_A_Non_Allow_Listed_Content_Type(string contentType) + { + var model = new ArtworkContentTypeModel("iptv/logos/abc", contentType); + + ArtworkContentTypeModel sanitized = model.Sanitized(); + + // Path is preserved; only the untrusted content type is neutralized. + sanitized.Path.ShouldBe("iptv/logos/abc"); + sanitized.ContentType.ShouldBe(string.Empty); + } + + [Test] + public void UrlWithContentType_Should_Be_Just_The_Path() + { + // The serve route no longer honors ?contentType= (issue #283), so the URL carries no query. + new ArtworkContentTypeModel("iptv/logos/abc", "image/png").UrlWithContentType.ShouldBe("iptv/logos/abc"); + } +} diff --git a/ErsatzTV.Tests/Application/Artworks/UploadArtworkHandlerTests.cs b/ErsatzTV.Tests/Application/Artworks/UploadArtworkHandlerTests.cs index 3a4792a90..35c8e05fd 100644 --- a/ErsatzTV.Tests/Application/Artworks/UploadArtworkHandlerTests.cs +++ b/ErsatzTV.Tests/Application/Artworks/UploadArtworkHandlerTests.cs @@ -14,6 +14,11 @@ namespace ErsatzTV.Tests.Application.Artworks; [TestFixture] public class UploadArtworkHandlerTests { + // A minimal valid 1x1 PNG. The handler derives the content type from bytes like these, never + // from a client-declared value (issue #283), so the tests exercise the real sniffer. + private static readonly byte[] PngBytes = Convert.FromBase64String( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+M8AAAMCAoAB/QwAAAAASUVORK5CYII="); + private IImageCache _imageCache = null!; private UploadArtworkHandler _handler = null!; @@ -25,17 +30,18 @@ public class UploadArtworkHandlerTests } [Test] - public async Task Handle_Should_Return_Logo_Path_With_Iptv_Logos_Prefix() + public async Task Handle_Should_Store_Logo_And_Return_Sniffed_ContentType() { _imageCache.SaveArtworkToCache(Arg.Any(), ArtworkKind.Logo) - .Returns(Right("abc123.png")); + .Returns(Right("abc123")); - using var stream = new MemoryStream(); + using var stream = new MemoryStream(PngBytes); Either result = - await _handler.Handle(new UploadArtwork(stream, "image/png", ArtworkKind.Logo), CancellationToken.None); + await _handler.Handle(new UploadArtwork(stream, ArtworkKind.Logo), CancellationToken.None); ArtworkUploadResponseModel response = RightOf(result); - response.Path.ShouldBe("iptv/logos/abc123.png"); + response.Path.ShouldBe("iptv/logos/abc123"); + // Content type comes from the actual PNG bytes, not from anything the client said. response.ContentType.ShouldBe("image/png"); } @@ -43,25 +49,41 @@ public class UploadArtworkHandlerTests public async Task Handle_Should_Return_Bare_File_Name_For_Watermark() { _imageCache.SaveArtworkToCache(Arg.Any(), ArtworkKind.Watermark) - .Returns(Right("def456.webp")); + .Returns(Right("def456")); - using var stream = new MemoryStream(); + using var stream = new MemoryStream(PngBytes); Either result = await _handler.Handle( - new UploadArtwork(stream, "image/webp", ArtworkKind.Watermark), + new UploadArtwork(stream, ArtworkKind.Watermark), CancellationToken.None); - RightOf(result).Path.ShouldBe("def456.webp"); + RightOf(result).Path.ShouldBe("def456"); } [Test] - public async Task Handle_Should_Reject_Unsupported_Content_Type() + public async Task Handle_Should_Reject_Non_Image_Payload_Spoofed_As_Image() + { + // The XSS chain: a "); + using var stream = new MemoryStream(htmlBytes); + + Either result = await _handler.Handle( + new UploadArtwork(stream, ArtworkKind.Logo), + CancellationToken.None); + + LeftOf(result).Value.ShouldContain("not a supported image"); + await _imageCache.DidNotReceive().SaveArtworkToCache(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task Handle_Should_Reject_Empty_Upload() { using var stream = new MemoryStream(); Either result = await _handler.Handle( - new UploadArtwork(stream, "image/bmp", ArtworkKind.Logo), + new UploadArtwork(stream, ArtworkKind.Logo), CancellationToken.None); - LeftOf(result).Value.ShouldContain("Unsupported image content type"); + LeftOf(result).Value.ShouldContain("not a supported image"); await _imageCache.DidNotReceive().SaveArtworkToCache(Arg.Any(), Arg.Any()); } @@ -71,9 +93,9 @@ public class UploadArtworkHandlerTests _imageCache.SaveArtworkToCache(Arg.Any(), ArtworkKind.Logo) .Returns(Left(BaseError.New("disk full"))); - using var stream = new MemoryStream(); + using var stream = new MemoryStream(PngBytes); Either result = await _handler.Handle( - new UploadArtwork(stream, "image/png", ArtworkKind.Logo), + new UploadArtwork(stream, ArtworkKind.Logo), CancellationToken.None); LeftOf(result).Value.ShouldBe("disk full"); diff --git a/ErsatzTV.Tests/Controllers/ArtworkUploadControllerTests.cs b/ErsatzTV.Tests/Controllers/ArtworkUploadControllerTests.cs index 8d2f44457..0c507ddba 100644 --- a/ErsatzTV.Tests/Controllers/ArtworkUploadControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/ArtworkUploadControllerTests.cs @@ -113,12 +113,13 @@ public class ArtworkUploadControllerTests var created = result.ShouldBeOfType(); created.StatusCode.ShouldBe(201); - created.Location.ShouldBe("/iptv/logos/abc.png?contentType=image%2Fpng"); + // No ?contentType= — the serve route sniffs the stored file (issue #283). + created.Location.ShouldBe("/iptv/logos/abc.png"); created.Value.ShouldBeOfType() .Path.ShouldBe("iptv/logos/abc.png"); await _mediator.Received(1).Send( - Arg.Is(c => c.ArtworkKind == ArtworkKind.Logo && c.ContentType == "image/png"), + Arg.Is(c => c.ArtworkKind == ArtworkKind.Logo), Arg.Any()); } @@ -133,7 +134,7 @@ public class ArtworkUploadControllerTests IActionResult result = await _controller.Upload(file, "watermark", CancellationToken.None); var created = result.ShouldBeOfType(); - created.Location.ShouldBe("/artwork/watermarks/def.webp?contentType=image%2Fwebp"); + created.Location.ShouldBe("/artwork/watermarks/def.webp"); await _mediator.Received(1).Send( Arg.Is(c => c.ArtworkKind == ArtworkKind.Watermark), diff --git a/ErsatzTV/Controllers/Api/ArtworkUploadController.cs b/ErsatzTV/Controllers/Api/ArtworkUploadController.cs index 487820a73..aead9f0f1 100644 --- a/ErsatzTV/Controllers/Api/ArtworkUploadController.cs +++ b/ErsatzTV/Controllers/Api/ArtworkUploadController.cs @@ -44,11 +44,11 @@ public class ArtworkUploadController(IMediator mediator) : ControllerBase await using Stream stream = file.OpenReadStream(); Either result = await mediator.Send( - new UploadArtwork(stream, file.ContentType, artworkKind), + new UploadArtwork(stream, artworkKind), cancellationToken); return result.ToCreatedResult( - value => LocationFor(artworkKind, value.Path, value.ContentType), + value => LocationFor(artworkKind, value.Path), value => value); } @@ -70,16 +70,13 @@ public class ArtworkUploadController(IMediator mediator) : ControllerBase } } - // Both GetImage (IptvController) and GetWatermark (ArtworkController) require a contentType - // query param to serve the cached file, so the Location header must carry it too. - private static string LocationFor(ArtworkKind artworkKind, string path, string contentType) - { - string encodedContentType = Uri.EscapeDataString(contentType); - return artworkKind switch + // The serve routes (GetImage / GetWatermark) sniff the content type from the stored file, so the + // Location no longer carries a ?contentType= (issue #283 — that reflection was the XSS sink). + private static string LocationFor(ArtworkKind artworkKind, string path) => + artworkKind switch { // logo paths already carry the servable prefix ("iptv/logos/{file}") - ArtworkKind.Logo => $"/{path}?contentType={encodedContentType}", - _ => $"/artwork/watermarks/{path}?contentType={encodedContentType}" + ArtworkKind.Logo => $"/{path}", + _ => $"/artwork/watermarks/{path}" }; - } } diff --git a/ErsatzTV/Controllers/Api/Requests/CreateChannelFromLineupRequest.cs b/ErsatzTV/Controllers/Api/Requests/CreateChannelFromLineupRequest.cs index 60df675b4..f901b72e6 100644 --- a/ErsatzTV/Controllers/Api/Requests/CreateChannelFromLineupRequest.cs +++ b/ErsatzTV/Controllers/Api/Requests/CreateChannelFromLineupRequest.cs @@ -32,7 +32,7 @@ public record CreateChannelFromLineupRequest( Number, Group, Categories, - Logo, + (Logo ?? ArtworkContentTypeModel.None).Sanitized(), IsEnabled, ShowInEpg, TemplateId, diff --git a/ErsatzTV/Controllers/Api/Requests/CreateChannelRequest.cs b/ErsatzTV/Controllers/Api/Requests/CreateChannelRequest.cs index c1fafb073..fb2d64e3a 100644 --- a/ErsatzTV/Controllers/Api/Requests/CreateChannelRequest.cs +++ b/ErsatzTV/Controllers/Api/Requests/CreateChannelRequest.cs @@ -45,7 +45,7 @@ public record CreateChannelRequest( Categories, FFmpegProfileId, SlugSeconds, - Logo, + (Logo ?? ArtworkContentTypeModel.None).Sanitized(), StreamSelectorMode, StreamSelector, PreferredAudioLanguageCode, diff --git a/ErsatzTV/Controllers/Api/Requests/CreateWatermarkRequest.cs b/ErsatzTV/Controllers/Api/Requests/CreateWatermarkRequest.cs index a5898e4d7..d5dbd80be 100644 --- a/ErsatzTV/Controllers/Api/Requests/CreateWatermarkRequest.cs +++ b/ErsatzTV/Controllers/Api/Requests/CreateWatermarkRequest.cs @@ -27,7 +27,7 @@ public record CreateWatermarkRequest( public CreateWatermark ToCommand() => new( Name, - new ArtworkContentTypeModel(Image ?? string.Empty, ImageContentType ?? string.Empty), + new ArtworkContentTypeModel(Image ?? string.Empty, ImageContentType ?? string.Empty).Sanitized(), Mode, ImageSource, Location, diff --git a/ErsatzTV/Controllers/Api/Requests/UpdateChannelRequest.cs b/ErsatzTV/Controllers/Api/Requests/UpdateChannelRequest.cs index f028084e6..1cce873f2 100644 --- a/ErsatzTV/Controllers/Api/Requests/UpdateChannelRequest.cs +++ b/ErsatzTV/Controllers/Api/Requests/UpdateChannelRequest.cs @@ -46,7 +46,7 @@ public record UpdateChannelRequest( Categories, FFmpegProfileId, SlugSeconds, - Logo, + (Logo ?? ArtworkContentTypeModel.None).Sanitized(), StreamSelectorMode, StreamSelector, PreferredAudioLanguageCode, diff --git a/ErsatzTV/Controllers/Api/Requests/UpdateWatermarkRequest.cs b/ErsatzTV/Controllers/Api/Requests/UpdateWatermarkRequest.cs index d999d42c1..ad2f49653 100644 --- a/ErsatzTV/Controllers/Api/Requests/UpdateWatermarkRequest.cs +++ b/ErsatzTV/Controllers/Api/Requests/UpdateWatermarkRequest.cs @@ -28,7 +28,7 @@ public record UpdateWatermarkRequest( new( id, Name, - new ArtworkContentTypeModel(Image ?? string.Empty, ImageContentType ?? string.Empty), + new ArtworkContentTypeModel(Image ?? string.Empty, ImageContentType ?? string.Empty).Sanitized(), Mode, ImageSource, Location, diff --git a/ErsatzTV/Controllers/ArtworkController.cs b/ErsatzTV/Controllers/ArtworkController.cs index 17d714cb9..2e8c9afc5 100644 --- a/ErsatzTV/Controllers/ArtworkController.cs +++ b/ErsatzTV/Controllers/ArtworkController.cs @@ -65,7 +65,7 @@ public class ArtworkController : ControllerBase { Either cachedImagePath = await _mediator.Send( - new GetCachedImagePath(fileName, ArtworkKind.Poster, string.Empty, 440), + new GetCachedImagePath(fileName, ArtworkKind.Poster, 440), cancellationToken); return cachedImagePath.Match( Left: _ => new NotFoundResult(), @@ -75,13 +75,11 @@ public class ArtworkController : ControllerBase [HttpGet("/artwork/watermarks/{fileName}")] public async Task GetWatermark( string fileName, - [FromQuery] - string contentType, CancellationToken cancellationToken) { Either cachedImagePath = await _mediator.Send( - new GetCachedImagePath(fileName, ArtworkKind.Watermark, contentType), + new GetCachedImagePath(fileName, ArtworkKind.Watermark), cancellationToken); return cachedImagePath.Match( Left: _ => new NotFoundResult(), @@ -92,7 +90,7 @@ public class ArtworkController : ControllerBase public async Task GetFanArt(string fileName, CancellationToken cancellationToken) { Either cachedImagePath = - await _mediator.Send(new GetCachedImagePath(fileName, ArtworkKind.FanArt, string.Empty), cancellationToken); + await _mediator.Send(new GetCachedImagePath(fileName, ArtworkKind.FanArt), cancellationToken); return cachedImagePath.Match( Left: _ => new NotFoundResult(), Right: r => new PhysicalFileResult(r.FileName, r.MimeType)); @@ -167,7 +165,7 @@ public class ArtworkController : ControllerBase { Either cachedImagePath = await _mediator.Send( - new GetCachedImagePath(fileName, ArtworkKind.Thumbnail, string.Empty, 220), + new GetCachedImagePath(fileName, ArtworkKind.Thumbnail, 220), cancellationToken); return cachedImagePath.Match( Left: _ => new NotFoundResult(), diff --git a/ErsatzTV/Controllers/IptvController.cs b/ErsatzTV/Controllers/IptvController.cs index d70ed71af..8868577d7 100644 --- a/ErsatzTV/Controllers/IptvController.cs +++ b/ErsatzTV/Controllers/IptvController.cs @@ -283,10 +283,10 @@ public class IptvController : StreamingControllerBase [HttpGet("iptv/logos/{fileName}")] [HttpHead("iptv/logos/{fileName}.jpg")] [HttpGet("iptv/logos/{fileName}.jpg")] - public async Task GetImage(string fileName, [FromQuery] string contentType) + public async Task GetImage(string fileName) { Either cachedImagePath = - await _mediator.Send(new GetCachedImagePath(fileName, ArtworkKind.Logo, contentType)); + await _mediator.Send(new GetCachedImagePath(fileName, ArtworkKind.Logo)); return cachedImagePath.Match( Left: _ => new NotFoundResult(), Right: r => new PhysicalFileResult(r.FileName, r.MimeType)); diff --git a/ErsatzTV/Program.cs b/ErsatzTV/Program.cs index 2bfcae1f0..02e1ada54 100644 --- a/ErsatzTV/Program.cs +++ b/ErsatzTV/Program.cs @@ -172,6 +172,11 @@ public class Program } options.AddServerHeader = false; + + // Bound the request body to the configured upload cap so an oversized (e.g. + // multipart artwork) body is rejected as it's read, before it is buffered + // (issue #283 S9 — the controller's file.Length check only fired after binding). + options.Limits.MaxRequestBodySize = (long)SystemEnvironment.MaximumUploadMb * 1024 * 1024; }) .UseContentRoot(BasePath)) .UseSerilog(); diff --git a/docs/api-conventions.md b/docs/api-conventions.md index eefbe63f7..705964b6f 100644 --- a/docs/api-conventions.md +++ b/docs/api-conventions.md @@ -216,6 +216,22 @@ Do **not** reuse the Application-layer Mappers used by Blazor (e.g. `MediaCards` mappers) for new API DTOs — those still return the old Blazor-convention relative paths. Map from the domain/VM directly and root the path yourself, following the PR #181 pattern. +### 4a. Artwork content type is sniffed, never client-supplied (issue #283) + +The uploaded-artwork surfaces (channel logo, watermark) must never trust a client-declared content +type — doing so was a stored-XSS chain (upload `