diff --git a/ErsatzTV.Infrastructure.Tests/Images/ImageSharpDecodingTests.cs b/ErsatzTV.Infrastructure.Tests/Images/ImageSharpDecodingTests.cs index b6c3ed362..08b15690b 100644 --- a/ErsatzTV.Infrastructure.Tests/Images/ImageSharpDecodingTests.cs +++ b/ErsatzTV.Infrastructure.Tests/Images/ImageSharpDecodingTests.cs @@ -4,6 +4,7 @@ using NUnit.Framework; using Shouldly; using SixLabors.ImageSharp; using SixLabors.ImageSharp.Formats; +using SixLabors.ImageSharp.Formats.Bmp; using SixLabors.ImageSharp.Formats.Jpeg; using SixLabors.ImageSharp.Formats.Png; using SixLabors.ImageSharp.Formats.Tiff; @@ -105,7 +106,31 @@ public class ImageSharpDecodingTests private const long HostileIccEntriesFloor = 64L * 1024 * 1024; internal const long ProductPathCeiling = 16L * 1024 * 1024; - internal static readonly string[] IccCarriers = ["png", "jpeg", "webp"]; + internal static readonly string[] IccCarriers = ["bmp", "jpeg", "png", "webp"]; + + // the carrier list is checked, not trusted: every format the decode configuration registers is + // round-tripped with the hostile profile through its default encoder, and the formats whose + // decode still carries a profile must be exactly IccCarriers. A version that adds ICC support to + // another format reddens here. A round trip cannot see a decoder that reads a profile its own + // encoder never writes; on 3.1.12 the decoders that construct an IccProfile are BMP, JPEG, PNG, + // WebP and TIFF (Cecil scan of the package, 2026-10-28), so none is missed today. + [Test] + public async Task Icc_Carriers_Are_Every_Configured_Format_That_Round_Trips_A_Profile() + { + var carriers = new List(); + foreach (IImageFormat format in ImageSharpDecoding.Configuration.ImageFormats) + { + IImageEncoder encoder = Configuration.Default.ImageFormatsManager.GetEncoder(format); + await using MemoryStream stream = await WithHostileIcc(encoder); + using Image image = await Image.LoadAsync(ImageSharpDecoding.Options(), stream); + if (image.Metadata.IccProfile is not null) + { + carriers.Add(format.Name.ToLowerInvariant()); + } + } + + carriers.Order(StringComparer.Ordinal).ShouldBe(IccCarriers.Order(StringComparer.Ordinal)); + } // positive control: the fixture really is hostile. Reading Entries runs the CLUT reader on the // declared 255-point, 3-input grid (about 127 MiB on 3.1.12, measured 2026-10-28); the catch @@ -165,19 +190,22 @@ public class ImageSharpDecodingTests /// mft2 (lut16) with 3 inputs, 15 outputs and 255 grid points, truncated before the /// CLUT: a 208-byte profile. /// - internal static async Task WithHostileIcc(string format) + internal static Task WithHostileIcc(string format) => + WithHostileIcc( + format switch + { + "bmp" => new BmpEncoder(), + "jpeg" => new JpegEncoder(), + "png" => new PngEncoder(), + "webp" => new WebpEncoder { FileFormat = WebpFileFormatType.Lossless }, + _ => throw new ArgumentOutOfRangeException(nameof(format), format, null) + }); + + private static async Task WithHostileIcc(IImageEncoder encoder) { using var image = new Image(16, 16); image.Metadata.IccProfile = new IccProfile(HostileIccBytes()); - IImageEncoder encoder = format switch - { - "png" => new PngEncoder(), - "jpeg" => new JpegEncoder(), - "webp" => new WebpEncoder { FileFormat = WebpFileFormatType.Lossless }, - _ => throw new ArgumentOutOfRangeException(nameof(format), format, null) - }; - var stream = new MemoryStream(); await image.SaveAsync(stream, encoder); stream.Position = 0; diff --git a/ErsatzTV.Infrastructure.Tests/Streaming/Graphics/WatermarkElementRemoteImageTests.cs b/ErsatzTV.Infrastructure.Tests/Streaming/Graphics/WatermarkElementRemoteImageTests.cs index 6ce16f489..c8d5f5c79 100644 --- a/ErsatzTV.Infrastructure.Tests/Streaming/Graphics/WatermarkElementRemoteImageTests.cs +++ b/ErsatzTV.Infrastructure.Tests/Streaming/Graphics/WatermarkElementRemoteImageTests.cs @@ -174,8 +174,10 @@ public class WatermarkElementRemoteImageTests // GHSA-gwg2-r3hj-4w44 over the element itself rather than a replica of its calls: decode, // CloneFrame, Resize and CloneAs carry a hostile ICC profile through without sizing its CLUT. - // The fixture's hostility is proven by ImageSharpDecodingTests' - // Hostile_Icc_Fixture_Allocates_When_Its_Entries_Are_Parsed. + // The element's images are private, so this case cannot assert the profile is still attached; it + // shares its fixture with ImageSharpDecodingTests, whose + // Decode_Clone_And_Resize_Do_Not_Parse_A_Hostile_Icc_Profile asserts attachment after each step and + // whose Hostile_Icc_Fixture_Allocates_When_Its_Entries_Are_Parsed proves the fixture hostile. // (ersatztv#929, security.imagesharp-3x-tiff-decoder-disabled) [TestCaseSource(typeof(ImageSharpDecodingTests), nameof(ImageSharpDecodingTests.IccCarriers))] [NonParallelizable] diff --git a/docs/decisions/records/security/imagesharp-3x-tiff-decoder-disabled.md b/docs/decisions/records/security/imagesharp-3x-tiff-decoder-disabled.md index 9e22ae10b..2164ee3e7 100644 --- a/docs/decisions/records/security/imagesharp-3x-tiff-decoder-disabled.md +++ b/docs/decisions/records/security/imagesharp-3x-tiff-decoder-disabled.md @@ -79,12 +79,19 @@ by `ImageSharpSurfaceTests`. The gwg2 runtime half is ImageSharp code an IL scan of ErsatzTV cannot see, so it is pinned by behaviour. A 208-byte ICC profile declares an `mft2` CLUT of 255 grid points over 3 inputs, and it is -carried in a PNG, a JPEG and a WebP. `ImageSharpDecodingTests` sends it through the validator plus -`CloneFrame`/`Resize`/`CloneAs`, and the watermark fixture through a real remote `WatermarkElement`. -Each fails above 16 MiB allocated, and each asserts that the profile is still attached, so a decoder -that drops it cannot pass vacuously. The positive control reads `Entries` on the same profile and -must allocate over 64 MiB (about 127 MiB on 3.1.12). So a version that starts parsing profiles on -decode or clone reddens. +carried in every format whose decoder reads a profile: BMP, JPEG, PNG and WebP. That carrier list is +checked rather than trusted: each format the decode configuration registers is round-tripped with +the profile through its default encoder, and the formats that come back carrying it must be exactly +the list, so a version that adds ICC to another format reddens. A round trip cannot see a decoder +that reads a profile its own encoder never writes; on 3.1.12 the decoders that construct an +`IccProfile` are BMP, JPEG, PNG, WebP and TIFF (Cecil scan of the package, 2026-10-28). +`ImageSharpDecodingTests` sends each carrier through the validator plus `CloneFrame`/`Resize`/`CloneAs`, +and the watermark fixture through a real remote `WatermarkElement`; both fail above 16 MiB allocated. +The validator cases assert that the profile is still attached after decode, `CloneFrame`, `Resize` +and `CloneAs`, so a decoder that drops it cannot pass them vacuously. The element case cannot see +its private images and asserts only the allocation; it shares the validator cases' fixture. The +positive control reads `Entries` on the same profile and must allocate over 64 MiB (about 127 MiB on +3.1.12). So a version that starts parsing profiles on decode or clone reddens. One shape is outside every check: a `Save(path)` that picks its encoder from the file extension. There is no ImageSharp encode in production code (2026-10-27).