From eec62283617d8e4a5d006cf84f1f99f180cbe263 Mon Sep 17 00:00:00 2001 From: npond Date: Sun, 30 Aug 2026 09:26:12 -0400 Subject: [PATCH] Images: refuse a zlib stream that asks for a preset dictionary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A zlib header with FDICT set makes zlib ask for a preset dictionary rather than call the stream corrupt, and .NET raises ZLibException for that — an IOException, outside the InvalidDataException family both image inflaters caught and outside every clause of the ImageReader net. Two bytes of a PNG IDAT or a TIFF Deflate strip therefore took the whole conversion down. Caught as IOException, because ZLibException cannot be named: it is public in System.IO.Compression but absent from the net10.0 reference assembly. The width costs nothing on these paths — the only stream is a MemoryStream over a byte array in hand, which performs no I/O, so an IOException there can only have come from the inflater. Tests at both decoders, where the ImageReader net cannot swallow them, plus libFuzzer's minimised unit through TryRead; all three fail without the fix. The unit is seeded into the image corpus, which lives in the Actions cache rather than in git. Found by the scheduled fuzz job, run 33303663302. Refs #296 --- fuzz/Program.cs | 9 ++++ src/n8PDF/Images/ImageReader.cs | 10 ++++- src/n8PDF/Images/PngDecoder.cs | 10 ++++- src/n8PDF/Images/TiffDecoder.cs | 5 ++- tests/n8PDF.Tests/ImageDecoderHostileTests.cs | 21 ++++++++++ tests/n8PDF.Tests/PngHostileTests.cs | 42 +++++++++++++++++++ 6 files changed, 93 insertions(+), 4 deletions(-) diff --git a/fuzz/Program.cs b/fuzz/Program.cs index 8612dbd..0d19729 100644 --- a/fuzz/Program.cs +++ b/fuzz/Program.cs @@ -186,6 +186,7 @@ private static void Seed() File.WriteAllBytes(Path.Combine(image, "minimal.png"), MinimalPng()); File.WriteAllBytes(Path.Combine(image, "gif-header"), "GIF89a"u8.ToArray()); File.WriteAllBytes(Path.Combine(image, "bmp-header"), "BM"u8.ToArray()); + File.WriteAllBytes(Path.Combine(image, "png-preset-dictionary"), PresetDictionaryPng()); File.WriteAllBytes(Path.Combine(image, "empty"), []); var font = Path.Combine(fixtures, "Fonts", "n8PDFProbe.ttf"); @@ -252,6 +253,14 @@ private static string RepositoryRoot() "could not find the repository root (a directory holding tests/n8PDF.Tests/Fixtures)"); } + /// + /// libFuzzer's minimised unit for #296, kept as a seed so a fresh corpus carries the + /// regression: a PNG whose IDAT sets the zlib header's FDICT bit, which zlib answers by + /// asking for a preset dictionary and .NET reports as a ZLibException — once an escaper. + /// + private static byte[] PresetDictionaryPng() => Convert.FromBase64String( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAAECAQAAAgAAAAAAAAADElEQVR4P5xjsmAcAAAARAAB//8dCFMA"); + private static byte[] MinimalPng() { var png = new List { 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A }; diff --git a/src/n8PDF/Images/ImageReader.cs b/src/n8PDF/Images/ImageReader.cs index bc25974..d48ee8a 100644 --- a/src/n8PDF/Images/ImageReader.cs +++ b/src/n8PDF/Images/ImageReader.cs @@ -57,7 +57,7 @@ public static ImageData Read( } catch (Exception e) when (e is ImageFormatException or IndexOutOfRangeException or ArgumentException or OverflowException - or DivideByZeroException or InvalidDataException) + or DivideByZeroException or InvalidDataException or IOException) { // A malformed image should cost its own placement, not the whole conversion — and // that sentence has to hold for the files the decoders did not think to refuse, not @@ -65,7 +65,13 @@ or IndexOutOfRangeException or ArgumentException or OverflowException // escaping from crafted files of a few dozen bytes; each hole is filed as its own // issue with its own validation fix, tested at the decoder level where this net // cannot swallow the evidence, and this is the defence in depth behind those fixes - // rather than a substitute for any of them. OutOfMemoryException is deliberately not + // rather than a substitute for any of them. IOException joined the list when the fuzz + // job found ZLibException escaping a PNG whose zlib header asks for a preset + // dictionary (#296): it derives from IOException rather than InvalidDataException, so + // it sat outside every clause here, and it cannot be named — it is public in + // System.IO.Compression but absent from the net10.0 reference assembly. The width + // costs nothing, because every decoder behind this reads a byte array already in + // memory and none of them opens a file. OutOfMemoryException is deliberately not // here: that one means the process is in trouble, and hiding it helps nobody. _ = e; return null; diff --git a/src/n8PDF/Images/PngDecoder.cs b/src/n8PDF/Images/PngDecoder.cs index c2bffee..8489f36 100644 --- a/src/n8PDF/Images/PngDecoder.cs +++ b/src/n8PDF/Images/PngDecoder.cs @@ -138,9 +138,17 @@ private static byte[] Inflate(byte[] compressed, long maxBytes) output.Write(buffer, 0, read); } } - catch (InvalidDataException) + catch (Exception e) when (e is InvalidDataException or IOException) { // A corrupt zlib stream is a picture this cannot read, not a fault in the reader (#7). + // IOException is the other half of that sentence: a header whose FDICT bit is set asks + // for a preset dictionary, which leaves zlib wanting one rather than calling the + // stream corrupt, and .NET raises ZLibException for it — outside + // InvalidDataException's family, and so outside the catch this once was (#296). + // ZLibException cannot be named: it is public in System.IO.Compression but absent + // from the net10.0 reference assembly, so its public base is what a catch can say. + // Nothing is lost by the width, because the only stream here is a MemoryStream over a + // byte array in hand — it performs no I/O, so an IOException can only be the inflater. throw new ImageFormatException("PNG image data is not a valid zlib stream."); } diff --git a/src/n8PDF/Images/TiffDecoder.cs b/src/n8PDF/Images/TiffDecoder.cs index b43ca9d..1972a19 100644 --- a/src/n8PDF/Images/TiffDecoder.cs +++ b/src/n8PDF/Images/TiffDecoder.cs @@ -601,9 +601,12 @@ private static byte[] Inflate(byte[] data, int offset, int length, int expected) output.Write(buffer, 0, read); } } - catch (InvalidDataException) + catch (Exception e) when (e is InvalidDataException or IOException) { // A corrupt Deflate strip is a picture this cannot read, not a fault in the reader (#35). + // A strip whose zlib header sets FDICT asks for a preset dictionary instead of being + // corrupt, and .NET raises ZLibException for that — the same hole as in PngDecoder, + // caught the same way and for the same reasons, which are written out there (#296). throw new ImageFormatException("TIFF strip is not a valid Deflate stream."); } diff --git a/tests/n8PDF.Tests/ImageDecoderHostileTests.cs b/tests/n8PDF.Tests/ImageDecoderHostileTests.cs index bc5890b..aee5efd 100644 --- a/tests/n8PDF.Tests/ImageDecoderHostileTests.cs +++ b/tests/n8PDF.Tests/ImageDecoderHostileTests.cs @@ -189,6 +189,27 @@ public void Tiff_deflate_strip_is_bounded_against_its_rows() // #30 Timed("TIFF deflate bomb", () => ImageReader.TryRead(tiff)); } + [Fact] + public void Tiff_deflate_strip_asking_for_a_preset_dictionary_fails_cleanly() // #296 + { + // The same hole as the PNG one: a strip whose zlib header sets FDICT leaves zlib wanting + // a dictionary, which .NET reports as ZLibException rather than a data error, and which + // #35's catch therefore let past. Asserted at the decoder, where the net cannot hide it. + byte[] strip = [0x78, 0x3f, 0x00, 0x00, 0x00, 0x01, 0x63, 0x00, 0x00, 0x00, 0x00]; + var b = new TiffBuilder().Tags(8); + var stripAt = b.Blob(strip); + var tiff = b + .Tag(256, 3, 1, 8).Tag(257, 3, 1, 8) // 8x8 + .Tag(258, 3, 1, 8).Tag(277, 3, 1, 1) + .Tag(259, 3, 1, 8) // Deflate + .Tag(273, 4, 1, stripAt).Tag(279, 4, 1, strip.Length) + .Build(); + _output.WriteLine($"{tiff.Length}-byte TIFF whose Deflate strip asks for a preset dictionary"); + + Assert.IsType(Record.Exception(() => TiffDecoder.Decode(tiff))); + Assert.Null(ImageReader.TryRead(tiff)); + } + [Fact] public void Ccitt_zero_run_strip_does_not_spin() // #46 { diff --git a/tests/n8PDF.Tests/PngHostileTests.cs b/tests/n8PDF.Tests/PngHostileTests.cs index ef44565..ce5c817 100644 --- a/tests/n8PDF.Tests/PngHostileTests.cs +++ b/tests/n8PDF.Tests/PngHostileTests.cs @@ -36,6 +36,15 @@ private static byte[] ZlibOfZeros(int count) return raw.ToArray(); } + /// A zlib stream whose header asks for a preset dictionary it cannot be given. + private static byte[] PresetDictionaryZlib() + { + // CMF 0x78 is deflate over a 32K window; FLG 0x3f sets FDICT (bit 5) and carries an + // FCHECK that makes the pair a multiple of 31, so the header passes its own check and + // zlib gets as far as asking. What follows stands in for the DICTID and the data. + return [0x78, 0x3f, 0x00, 0x00, 0x00, 0x01, 0x63, 0x00, 0x00, 0x00, 0x00]; + } + [Fact] public void A_decompression_bomb_is_refused_against_the_declared_size() { @@ -121,6 +130,39 @@ public void A_corrupt_idat_fails_cleanly() // #7 Assert.Null(ImageReader.TryRead(png.ToArray())); } + [Fact] + public void An_idat_asking_for_a_preset_dictionary_fails_cleanly() // #296 + { + // FDICT is the one malformation zlib answers with Z_NEED_DICT rather than a data error, + // and .NET raises ZLibException for it — an IOException, outside the family #7's catch + // named. Two bytes of a PNG therefore used to take the whole conversion down. + var png = new List { 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A }; + var ihdr = new List(); + Be32(ihdr, 2); Be32(ihdr, 2); + ihdr.Add(8); ihdr.Add(6); ihdr.Add(0); ihdr.Add(0); ihdr.Add(0); + Chunk(png, "IHDR", ihdr.ToArray()); + Chunk(png, "IDAT", PresetDictionaryZlib()); + Chunk(png, "IEND", []); + + var ex = Record.Exception(() => PngDecoder.Decode(png.ToArray())); + Assert.IsType(ex); + Assert.Null(ImageReader.TryRead(png.ToArray())); + } + + [Fact] + public void The_minimised_fuzz_unit_from_the_image_target_is_read_as_null() // #296 + { + // libFuzzer's own reduction of the crash, kept verbatim: the input the scheduled job + // wrote out. The corpus itself is not in git — it lives in the Actions cache and is + // rebuilt by `dotnet run -- seed` — so fuzz/Program.cs seeds this same unit, and this + // holds the assertion where the suite can see it. + var unit = Convert.FromBase64String( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAAECAQAAAgAAAAAAAAADElEQVR4P5xjsmAcAAAARAAB//8dCFMA"); + _output.WriteLine($"the minimised unit is {unit.Length} bytes"); + + Assert.Null(ImageReader.TryRead(unit)); + } + [Fact] public void A_chunk_length_near_int_max_does_not_pass_the_bounds_check() // #3 {