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 {