test(929): the ICC carrier list includes BMP and is checked against a round trip of every configured format
Build ErsatzTV Image / CI toolchain image resolves (pull_request) Successful in 9s
Build ErsatzTV Image / Delimiter ban (release path) (pull_request) Successful in 20s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 9s
PR Gates / Docs update reminder (pull_request) Successful in 11s
PR Gates / decisions lifecycle (pull_request) Successful in 14s
PR Gates / Fix proofs (Proves trailers) (pull_request) Successful in 10s
Review verdict / Set review-verdict status (pull_request_target) Successful in 16s
PR Gates / Script lint and tests (ruff + pytest) (pull_request) Successful in 12m42s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 8m37s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 6m0s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Skipped
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 5m56s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 8s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 7s
review-verdict/h10 Review-verdict: MERGEABLE @ 71d660f (base: main)

Round 4 review found two gaps in the gwg2 runtime pin. The carrier list
(png, jpeg, webp) was hand-written and left out BMP, whose 3.1.12
decoder also constructs an IccProfile (BmpDecoderCore::ReadInfoHeader)
and which remote-URL and on-disk graphics can reach. And the record
said every hostile-ICC case asserts the profile is still attached,
while the WatermarkElement case asserts only allocation (its images are
private).

- IccCarriers is now bmp, jpeg, png, webp, with a BmpEncoder arm.
- Icc_Carriers_Are_Every_Configured_Format_That_Round_Trips_A_Profile
  round-trips the hostile profile through each format of
  ImageSharpDecoding.Configuration via its default encoder and requires
  the formats that come back carrying a profile to equal IccCarriers.
  Witnessed red by dropping "bmp" from the list.
- A Cecil scan of the 3.1.12 package (2026-10-28) lists the decoders that
  construct an IccProfile as BMP, JPEG, PNG, WebP and TIFF, so the
  round trip's blind spot (a decoder reading a profile its encoder never
  writes) is empty today; the record says so, dated.
- The record and the element test's comment now say only the validator
  cases assert attachment; the element case shares their fixture.

Measured: product path 38-94 KiB, element 368-834 KiB, Entries control
about 129,545 KiB for all four carriers.

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: f9ce6bc6-5b36-491a-8d72-89c1f70dd3e1
This commit is contained in:
2026-10-30 00:03:25 +01:00
co-authored by Claude Opus 5.5
parent bed50e2b77
commit 71d660f213
3 changed files with 55 additions and 18 deletions
@@ -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<string>();
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
/// <c>mft2</c> (lut16) with 3 inputs, 15 outputs and 255 grid points, truncated before the
/// CLUT: a 208-byte profile.
/// </summary>
internal static async Task<MemoryStream> WithHostileIcc(string format)
internal static Task<MemoryStream> 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<MemoryStream> WithHostileIcc(IImageEncoder encoder)
{
using var image = new Image<Rgba32>(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;
@@ -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]
@@ -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).