From fdc15d283ec21dac7df993d4795389a4bab74255 Mon Sep 17 00:00:00 2001 From: Erik White <26148654+Erik-White@users.noreply.github.com> Date: Wed, 27 May 2026 10:58:59 +0200 Subject: [PATCH] Separate Zlib header validation fom stream reading --- .../Compression/Zlib/ChunkedReadStream.cs | 125 ++++++++++ .../Compression/Zlib/ZlibInflateStream.cs | 219 ++---------------- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 26 ++- 3 files changed, 167 insertions(+), 203 deletions(-) create mode 100644 src/ImageSharp/Compression/Zlib/ChunkedReadStream.cs diff --git a/src/ImageSharp/Compression/Zlib/ChunkedReadStream.cs b/src/ImageSharp/Compression/Zlib/ChunkedReadStream.cs new file mode 100644 index 0000000000..8c5189d4a9 --- /dev/null +++ b/src/ImageSharp/Compression/Zlib/ChunkedReadStream.cs @@ -0,0 +1,125 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using SixLabors.ImageSharp.IO; + +namespace SixLabors.ImageSharp.Compression.Zlib; + +/// +/// A read-only stream over a sequence of length-delimited segments. Bytes are +/// pulled from the inner stream up to the current segment's remaining length; +/// when the segment is exhausted the supplied delegate is invoked to advance +/// to the next segment and return its length. The inner stream is not owned +/// and is not disposed. +/// +internal sealed class ChunkedReadStream : Stream +{ + private static readonly Func GetDataNoOp = () => 0; + + private readonly BufferedReadStream innerStream; + private readonly Func getData; + private int currentDataRemaining; + + public ChunkedReadStream(BufferedReadStream innerStream) + : this(innerStream, GetDataNoOp) + { + } + + public ChunkedReadStream(BufferedReadStream innerStream, Func getData) + { + this.innerStream = innerStream; + this.getData = getData; + } + + /// + public override bool CanRead => this.innerStream.CanRead; + + /// + public override bool CanSeek => false; + + /// + public override bool CanWrite => throw new NotSupportedException(); + + /// + public override long Length => throw new NotSupportedException(); + + /// + public override long Position { get => throw new NotSupportedException(); set => throw new NotSupportedException(); } + + /// + /// Sets the number of bytes available to read from the current segment. + /// Must be called before reading each segment. + /// + public void SetCurrentSegmentLength(int bytes) => this.currentDataRemaining = bytes; + + /// + public override void Flush() => throw new NotSupportedException(); + + /// + public override int ReadByte() + { + this.currentDataRemaining--; + return this.innerStream.ReadByte(); + } + + /// + public override int Read(byte[] buffer, int offset, int count) + { + if (this.currentDataRemaining is 0) + { + // Current segment is exhausted; ask the caller for the next one. + this.currentDataRemaining = this.getData(); + + if (this.currentDataRemaining is 0) + { + return 0; + } + } + + int bytesToRead = Math.Min(count, this.currentDataRemaining); + this.currentDataRemaining -= bytesToRead; + int totalBytesRead = this.innerStream.Read(buffer, offset, bytesToRead); + long innerStreamLength = this.innerStream.Length; + + // Keep reading data until we've reached the end of the stream or filled the buffer. + int bytesRead = 0; + offset += totalBytesRead; + while (this.currentDataRemaining is 0 && totalBytesRead < count) + { + this.currentDataRemaining = this.getData(); + + if (this.currentDataRemaining is 0) + { + return totalBytesRead; + } + + offset += bytesRead; + + if (offset >= innerStreamLength || offset >= count) + { + return totalBytesRead; + } + + bytesToRead = Math.Min(count - totalBytesRead, this.currentDataRemaining); + this.currentDataRemaining -= bytesToRead; + bytesRead = this.innerStream.Read(buffer, offset, bytesToRead); + if (bytesRead == 0) + { + return totalBytesRead; + } + + totalBytesRead += bytesRead; + } + + return totalBytesRead; + } + + /// + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + + /// + public override void SetLength(long value) => throw new NotSupportedException(); + + /// + public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); +} diff --git a/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs b/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs index 513171b179..11f34dac8a 100644 --- a/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs +++ b/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs @@ -8,9 +8,11 @@ using SixLabors.ImageSharp.IO; namespace SixLabors.ImageSharp.Compression.Zlib; /// -/// Provides methods and properties for deframing streams from PNGs. +/// Reads chunked input, parses the zlib CMF/FLG header, and exposes a +/// over the remaining DEFLATE payload. The +/// Adler-32 trailer is not validated. /// -internal sealed class ZlibInflateStream : Stream +internal sealed class ZlibInflateStream : IDisposable { /// /// Used to read the Adler-32 and Crc-32 checksums. @@ -19,94 +21,13 @@ internal sealed class ZlibInflateStream : Stream /// private static readonly byte[] ChecksumBuffer = new byte[4]; - /// - /// A default delegate to get more data from the inner stream. - /// - private static readonly Func GetDataNoOp = () => 0; - - /// - /// The inner raw memory stream. - /// - private readonly BufferedReadStream innerStream; - - /// - /// A value indicating whether this instance of the given entity has been disposed. - /// - /// if this instance has been disposed; otherwise, . - /// - /// If the entity is disposed, it must not be disposed a second - /// time. The isDisposed field is set the first time the entity - /// is disposed. If the isDisposed field is true, then the Dispose() - /// method will not dispose again. This help not to prolong the entity's - /// life in the Garbage Collector. - /// - private bool isDisposed; - - /// - /// The current data remaining to be read. - /// - private int currentDataRemaining; - - /// - /// Delegate to get more data once we've exhausted the current data remaining. - /// - private readonly Func getData; - - /// - /// When true, the inflated payload is treated as a raw DEFLATE stream with no zlib - /// CMF/FLG header (and no Adler-32 trailer). This is required to decode IDATs in - /// Apple's proprietary CgBI PNG variant. - /// - private readonly bool noHeader; + private readonly ChunkedReadStream segmentStream; - /// - /// Initializes a new instance of the class. - /// - /// The inner raw stream. public ZlibInflateStream(BufferedReadStream innerStream) - : this(innerStream, GetDataNoOp, noHeader: false) - { - } + => this.segmentStream = new ChunkedReadStream(innerStream); - /// - /// Initializes a new instance of the class. - /// - /// The inner raw stream. - /// A delegate to get more data from the inner stream. public ZlibInflateStream(BufferedReadStream innerStream, Func getData) - : this(innerStream, getData, noHeader: false) - { - } - - /// - /// Initializes a new instance of the class. - /// - /// The inner raw stream. - /// A delegate to get more data from the inner stream. - /// - /// When , the payload is treated as raw DEFLATE with no zlib header. - /// - public ZlibInflateStream(BufferedReadStream innerStream, Func getData, bool noHeader) - { - this.innerStream = innerStream; - this.getData = getData; - this.noHeader = noHeader; - } - - /// - public override bool CanRead => this.innerStream.CanRead; - - /// - public override bool CanSeek => false; - - /// - public override bool CanWrite => throw new NotSupportedException(); - - /// - public override long Length => throw new NotSupportedException(); - - /// - public override long Position { get => throw new NotSupportedException(); set => throw new NotSupportedException(); } + => this.segmentStream = new ChunkedReadStream(innerStream, getData); /// /// Gets the compressed stream over the deframed inner stream. @@ -114,15 +35,16 @@ internal sealed class ZlibInflateStream : Stream public DeflateStream? CompressedStream { get; private set; } /// - /// Adds new bytes from a frame found in the original stream. + /// Sets the length of the next segment of compressed input and, on first + /// call, parses the zlib header. /// - /// The current remaining data according to the chunk length. - /// Whether the chunk to be inflated is a critical chunk. + /// The remaining data length for the current segment. + /// Whether to throw on a malformed zlib header. /// The . [MemberNotNullWhen(true, nameof(CompressedStream))] public bool AllocateNewBytes(int bytes, bool isCriticalChunk) { - this.currentDataRemaining = bytes; + this.segmentStream.SetCurrentSegmentLength(bytes); if (this.CompressedStream is null) { return this.InitializeInflateStream(isCriticalChunk); @@ -131,114 +53,15 @@ internal sealed class ZlibInflateStream : Stream return true; } - /// - public override void Flush() => throw new NotSupportedException(); - - /// - public override int ReadByte() + public void Dispose() { - this.currentDataRemaining--; - return this.innerStream.ReadByte(); - } - - /// - public override int Read(byte[] buffer, int offset, int count) - { - if (this.currentDataRemaining is 0) - { - // Last buffer was read in its entirety, let's make sure we don't actually have more in additional IDAT chunks. - this.currentDataRemaining = this.getData(); - - if (this.currentDataRemaining is 0) - { - return 0; - } - } - - int bytesToRead = Math.Min(count, this.currentDataRemaining); - this.currentDataRemaining -= bytesToRead; - int totalBytesRead = this.innerStream.Read(buffer, offset, bytesToRead); - long innerStreamLength = this.innerStream.Length; - - // Keep reading data until we've reached the end of the stream or filled the buffer. - int bytesRead = 0; - offset += totalBytesRead; - while (this.currentDataRemaining is 0 && totalBytesRead < count) - { - this.currentDataRemaining = this.getData(); - - if (this.currentDataRemaining is 0) - { - return totalBytesRead; - } - - offset += bytesRead; - - if (offset >= innerStreamLength || offset >= count) - { - return totalBytesRead; - } - - bytesToRead = Math.Min(count - totalBytesRead, this.currentDataRemaining); - this.currentDataRemaining -= bytesToRead; - bytesRead = this.innerStream.Read(buffer, offset, bytesToRead); - if (bytesRead == 0) - { - return totalBytesRead; - } - - totalBytesRead += bytesRead; - } - - return totalBytesRead; - } - - /// - public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); - - /// - public override void SetLength(long value) => throw new NotSupportedException(); - - /// - public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); - - /// - protected override void Dispose(bool disposing) - { - if (this.isDisposed) - { - return; - } - - if (disposing) - { - // Dispose managed resources. - if (this.CompressedStream != null) - { - this.CompressedStream.Dispose(); - this.CompressedStream = null; - } - } - - base.Dispose(disposing); - - // Call the appropriate methods to clean up - // unmanaged resources here. - // Note disposing is done. - this.isDisposed = true; + this.CompressedStream?.Dispose(); + this.segmentStream?.Dispose(); } [MemberNotNullWhen(true, nameof(CompressedStream))] private bool InitializeInflateStream(bool isCriticalChunk) { - // Apple CgBI IDATs omit the zlib CMF/FLG header and the Adler-32 trailer, - // wrapping a raw DEFLATE payload directly. Skip the header parsing in that mode. - if (this.noHeader) - { - this.CompressedStream = new DeflateStream(this, CompressionMode.Decompress, true); - return true; - } - // Read the zlib header : http://tools.ietf.org/html/rfc1950 // CMF(Compression Method and flags) // This byte is divided into a 4 - bit compression method and a @@ -250,9 +73,8 @@ internal sealed class ZlibInflateStream : Stream // +---+---+ // |CMF|FLG| // +---+---+ - int cmf = this.innerStream.ReadByte(); - int flag = this.innerStream.ReadByte(); - this.currentDataRemaining -= 2; + int cmf = this.segmentStream.ReadByte(); + int flag = this.segmentStream.ReadByte(); if (cmf == -1 || flag == -1) { return false; @@ -290,16 +112,13 @@ internal sealed class ZlibInflateStream : Stream { // We don't need this for inflate so simply skip by the next four bytes. // https://tools.ietf.org/html/rfc1950#page-6 - if (this.innerStream.Read(ChecksumBuffer, 0, 4) != 4) + if (this.segmentStream.Read(ChecksumBuffer, 0, 4) != 4) { return false; } - - this.currentDataRemaining -= 4; } - // Initialize the deflate BufferedReadStream. - this.CompressedStream = new DeflateStream(this, CompressionMode.Decompress, true); + this.CompressedStream = new DeflateStream(this.segmentStream, CompressionMode.Decompress, leaveOpen: true); return true; } diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 0d93429bb8..49b6fabc60 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -767,7 +767,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore /// The length of the chunk that containing the compressed scanline data. /// The pixel data. /// The png metadata - /// A delegate to get more data from the inner stream for . + /// A delegate to get more data from the inner stream when chunk boundaries are crossed. /// The frame control /// The cancellation token. private void ReadScanlines( @@ -779,14 +779,34 @@ internal sealed class PngDecoderCore : ImageDecoderCore CancellationToken cancellationToken) where TPixel : unmanaged, IPixel { - using ZlibInflateStream inflateStream = new(this.currentStream, getData, noHeader: this.isCgbi); + // CgBI IDATs wrap a raw DEFLATE payload directly (no zlib CMF/FLG header + // and no Adler-32 trailer); skip the zlib header parser entirely. + if (this.isCgbi) + { + using ChunkedReadStream segmentStream = new(this.currentStream, getData); + segmentStream.SetCurrentSegmentLength(chunkLength); + using DeflateStream cgbiDataStream = new(segmentStream, CompressionMode.Decompress, leaveOpen: true); + this.DecodeFromDeflate(cgbiDataStream, image, pngMetadata, frameControl, cancellationToken); + return; + } + + using ZlibInflateStream inflateStream = new(this.currentStream, getData); if (!inflateStream.AllocateNewBytes(chunkLength, !this.hasImageData)) { return; } - DeflateStream dataStream = inflateStream.CompressedStream!; + this.DecodeFromDeflate(inflateStream.CompressedStream!, image, pngMetadata, frameControl, cancellationToken); + } + private void DecodeFromDeflate( + DeflateStream dataStream, + ImageFrame image, + PngMetadata pngMetadata, + in FrameControl frameControl, + CancellationToken cancellationToken) + where TPixel : unmanaged, IPixel + { if (this.header.InterlaceMethod is PngInterlaceMode.Adam7) { this.DecodeInterlacedPixelData(frameControl, dataStream, image, pngMetadata, cancellationToken);