From ad4d01954ee0eb12aabca349e45c61f0f693609e Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 24 Aug 2026 19:24:22 +1000 Subject: [PATCH] Bound HEIF box parsing --- HEIF_IMPLEMENTATION_PLAN.md | 1 + .../Formats/Heif/HeifDecoderCore.cs | 148 +++++++++++++----- .../Formats/Heif/HeifDecoderTests.cs | 139 ++++++++++++++++ 3 files changed, 252 insertions(+), 36 deletions(-) diff --git a/HEIF_IMPLEMENTATION_PLAN.md b/HEIF_IMPLEMENTATION_PLAN.md index 15de2638a2..49b2995e33 100644 --- a/HEIF_IMPLEMENTATION_PLAN.md +++ b/HEIF_IMPLEMENTATION_PLAN.md @@ -64,6 +64,7 @@ This snapshot pins or classifies the available references and failures; it does | `Av1Inverse2dTransformer` and `Av1InverseTransformerFactory` | AV1 section 7.11.2 inverse transform and reconstruction | libaom `av1/common/av1_inv_txfm1d.c`, `av1/common/av1_inv_txfm2d.c`, and `av1/common/idct.c` at `03087864cf4bea6abb0d28f95cf7843511413d8f` | Scalar transform oracle for coefficient-row traversal, intermediate layout, stage ranges, clipping, and high-bit-depth sample addition. The managed 16-bit overload is also used as a parity oracle for the byte overload. | | `HeifDecoderCore` box extension handling and `HeifDecoderCore`/`HeifEncoderCore` item-property associations | ISO/IEC 14496-12 box extensibility and section 8.11.14 item properties and `ipma` syntax | libavif `src/read.c` and `src/write.c` at `092276ce89098ead06db80975173191e5fee1826` | Skip unrecognized top-level and metadata child boxes, preserve the position of every property in `ipco`, reject an unrecognized property only when its item association marks it essential, associate properties by item ID, and read or write the essential bit plus one-based 7-bit or 15-bit property index according to the full-box flags. Independent HEIC, HIF, and AVIF fixtures provide the reader oracle; container-level identification of encoded output guards the writer independently of pixel roundtripping. | | `HeifConstants.IsSupportedFileType`, `HeifImageFormatDetector`, and `HeifDecoderCore.CheckFileTypeBox` | ISO/IEC 14496-12 `FileTypeBox` syntax and the MP4 Registration Authority HEIF/AVIF still-image and sequence brand registrations | libavif `src/read.c` functions `avifParseFileTypeBox`, `avifFileTypeHasBrand`, and `avifFileTypeIsCompatible` at `092276ce89098ead06db80975173191e5fee1826` | Apply one rule to the major and compatible brands, accept the implemented still-image container and payload brands, and reject registered HEVC, AVIF, and JPEG sequence major brands until sequence decoding is implemented. The decoder validates the complete `ftyp` payload; the fixed-size format detector inspects the available prefix. | +| `HeifDecoderCore.ReadBoxHeader` and `HeifDecoderCore.ParseBoxHeader` | ISO/IEC 14496-12 section 4.2.2 basic box syntax | libavif `src/stream.c` functions `avifROStreamReadBoxHeaderPartial` and `avifROStreamReadBoxHeader` at `092276ce89098ead06db80975173191e5fee1826` | Resolve 32-bit, 64-bit, UUID, and top-level size-zero boxes into content lengths only after validating the complete variable-sized header and the remaining parent boundary. Nested size-zero boxes are invalid; large skips retain 64-bit offsets. | This table is intentionally incomplete. Add a row before each additional AV1 or HEVC algorithm is ported or materially reshaped. diff --git a/src/ImageSharp/Formats/Heif/HeifDecoderCore.cs b/src/ImageSharp/Formats/Heif/HeifDecoderCore.cs index b60e079011..ffd070b1b2 100644 --- a/src/ImageSharp/Formats/Heif/HeifDecoderCore.cs +++ b/src/ImageSharp/Formats/Heif/HeifDecoderCore.cs @@ -65,8 +65,7 @@ internal sealed class HeifDecoderCore : ImageDecoderCore Image? image = null; while (stream.Position < stream.Length) { - long boxLength = this.ReadBoxHeader(stream, out Heif4CharCode boxType); - EnsureBoxBoundary(boxLength, stream); + long boxLength = this.ReadBoxHeader(stream, stream.Length, out Heif4CharCode boxType, true); switch (boxType) { case Heif4CharCode.Meta: @@ -80,7 +79,7 @@ internal sealed class HeifDecoderCore : ImageDecoderCore break; case 0U: // Some files have trailing zeros, skiping to EOF. - stream.Skip((int)(stream.Length - stream.Position)); + SkipBox(stream, stream.Length - stream.Position); break; default: SkipBox(stream, boxLength); @@ -113,8 +112,7 @@ internal sealed class HeifDecoderCore : ImageDecoderCore while (stream.Position < stream.Length) { - long boxLength = this.ReadBoxHeader(stream, out Heif4CharCode boxType); - EnsureBoxBoundary(boxLength, stream); + long boxLength = this.ReadBoxHeader(stream, stream.Length, out Heif4CharCode boxType, true); switch (boxType) { case Heif4CharCode.Meta: @@ -140,13 +138,12 @@ internal sealed class HeifDecoderCore : ImageDecoderCore private bool CheckFileTypeBox(BufferedReadStream stream) { - long boxLength = this.ReadBoxHeader(stream, out Heif4CharCode boxType); + long boxLength = this.ReadBoxHeader(stream, stream.Length, out Heif4CharCode boxType, true); if (boxType != Heif4CharCode.Ftyp) { return false; } - EnsureBoxBoundary(boxLength, stream); if (boxLength < 8 || boxLength > int.MaxValue || (boxLength & 3) != 0) { return false; @@ -173,50 +170,126 @@ internal sealed class HeifDecoderCore : ImageDecoderCore meta.CompressionMethod = compressionMethod; } - private long ReadBoxHeader(BufferedReadStream stream, out Heif4CharCode boxType) + private long ReadBoxHeader(BufferedReadStream stream, long parentEndPosition, out Heif4CharCode boxType, bool topLevel = false) { - // Read 4 bytes of length of box + if (parentEndPosition - stream.Position < 8) + { + throw new InvalidImageContentException("Not enough data to read the box header."); + } + Span buf = stackalloc byte[8]; int bytesRead = stream.Read(buf); if (bytesRead != 8) { - throw new InvalidImageContentException("Not enough data to read the Box header"); + throw new InvalidImageContentException("Not enough data to read the box header."); } - long boxSize = BinaryPrimitives.ReadUInt32BigEndian(buf); - long headerSize = 8; - - // Read 4 bytes of box type + ulong boxSize = BinaryPrimitives.ReadUInt32BigEndian(buf); + int headerSize = 8; boxType = (Heif4CharCode)BinaryPrimitives.ReadUInt32BigEndian(buf[4..]); if (boxSize == 1) { + if (parentEndPosition - stream.Position < 8) + { + throw new InvalidImageContentException("Not enough data to read the extended box size."); + } + bytesRead = stream.Read(buf); if (bytesRead != 8) { - throw new InvalidImageContentException("Not enough data to read the Large Box header"); + throw new InvalidImageContentException("Not enough data to read the extended box size."); } - boxSize = (long)BinaryPrimitives.ReadUInt64BigEndian(buf); + boxSize = BinaryPrimitives.ReadUInt64BigEndian(buf); headerSize += 8; } - return boxSize - headerSize; + if (boxType == Heif4CharCode.Uuid) + { + if (parentEndPosition - stream.Position < 16) + { + throw new InvalidImageContentException("Not enough data to read the UUID box user type."); + } + + // The UUID user type is part of the variable-sized box header, even though this decoder skips its value. + SkipBox(stream, 16); + headerSize += 16; + } + + if (boxSize == 0) + { + // ISO BMFF permits a size-zero box only at file level, where it consumes the rest of the file. + if (!topLevel) + { + throw new InvalidImageContentException("A nested box cannot extend to the end of the file."); + } + + return parentEndPosition - stream.Position; + } + + if (boxSize < (ulong)headerSize) + { + throw new InvalidImageContentException("Box size is smaller than its header."); + } + + ulong contentLength = boxSize - (ulong)headerSize; + if (contentLength > (ulong)(parentEndPosition - stream.Position)) + { + throw new InvalidImageContentException("Box size extends beyond its parent boundary."); + } + + return (long)contentLength; } private static int ParseBoxHeader(Span buffer, out long length, out Heif4CharCode boxType) { - long boxSize = BinaryPrimitives.ReadUInt32BigEndian(buffer); - int bytesRead = 4; - boxType = (Heif4CharCode)BinaryPrimitives.ReadUInt32BigEndian(buffer[bytesRead..]); - bytesRead += 4; + if (buffer.Length < 8) + { + throw new InvalidImageContentException("Not enough data to read the box header."); + } + + ulong boxSize = BinaryPrimitives.ReadUInt32BigEndian(buffer); + int bytesRead = 8; + boxType = (Heif4CharCode)BinaryPrimitives.ReadUInt32BigEndian(buffer[4..]); if (boxSize == 1) { - boxSize = (long)BinaryPrimitives.ReadUInt64BigEndian(buffer[bytesRead..]); + if (buffer.Length < 16) + { + throw new InvalidImageContentException("Not enough data to read the extended box size."); + } + + boxSize = BinaryPrimitives.ReadUInt64BigEndian(buffer[bytesRead..]); bytesRead += 8; } - length = boxSize - bytesRead; + if (boxType == Heif4CharCode.Uuid) + { + if (buffer.Length - bytesRead < 16) + { + throw new InvalidImageContentException("Not enough data to read the UUID box user type."); + } + + bytesRead += 16; + } + + if (boxSize == 0) + { + throw new InvalidImageContentException("A nested box cannot extend to the end of the file."); + } + + if (boxSize < (ulong)bytesRead) + { + throw new InvalidImageContentException("Box size is smaller than its header."); + } + + ulong contentLength = boxSize - (ulong)bytesRead; + if (contentLength > (ulong)(buffer.Length - bytesRead)) + { + throw new InvalidImageContentException("Box size extends beyond its parent boundary."); + } + + length = (long)contentLength; return bytesRead; } @@ -226,8 +299,7 @@ internal sealed class HeifDecoderCore : ImageDecoderCore stream.Skip(4); while (stream.Position < endPosition) { - long length = this.ReadBoxHeader(stream, out Heif4CharCode boxType); - EnsureBoxInsideParent(length, boxLength); + long length = this.ReadBoxHeader(stream, endPosition, out Heif4CharCode boxType); switch (boxType) { case Heif4CharCode.Iprp: @@ -416,8 +488,7 @@ internal sealed class HeifDecoderCore : ImageDecoderCore long endBoxPosition = stream.Position + boxLength; while (stream.Position < endBoxPosition) { - long containerLength = this.ReadBoxHeader(stream, out Heif4CharCode containerType); - EnsureBoxInsideParent(containerLength, boxLength); + long containerLength = this.ReadBoxHeader(stream, endBoxPosition, out Heif4CharCode containerType); if (containerType == Heif4CharCode.Ipco) { // Parse Item Property Container, which is just an array of property boxes. @@ -440,8 +511,7 @@ internal sealed class HeifDecoderCore : ImageDecoderCore long endPosition = stream.Position + boxLength; while (stream.Position < endPosition) { - int itemLength = (int)this.ReadBoxHeader(stream, out Heif4CharCode itemType); - EnsureBoxInsideParent(itemLength, boxLength); + long itemLength = this.ReadBoxHeader(stream, endPosition, out Heif4CharCode itemType); using IMemoryOwner boxMemory = this.ReadIntoBuffer(stream, itemLength); Span boxBuffer = boxMemory.GetSpan(); switch (itemType) @@ -474,7 +544,7 @@ internal sealed class HeifDecoderCore : ImageDecoderCore Heif4CharCode profileType = (Heif4CharCode)BinaryPrimitives.ReadUInt32BigEndian(boxBuffer); if (profileType is Heif4CharCode.RICC or Heif4CharCode.Prof) { - byte[] iccData = new byte[itemLength - 4]; + byte[] iccData = new byte[(int)itemLength - 4]; boxBuffer[4..].CopyTo(iccData); this.metadata.IccProfile = new IccProfile(iccData); } @@ -762,15 +832,21 @@ internal sealed class HeifDecoderCore : ImageDecoderCore } private static void SkipBox(Stream stream, long boxLength) - => stream.Skip((int)boxLength); + => stream.Seek(boxLength, SeekOrigin.Current); private IMemoryOwner ReadIntoBuffer(Stream stream, long length) { - IMemoryOwner buffer = this.configuration.MemoryAllocator.Allocate((int)length); + if ((ulong)length > int.MaxValue) + { + throw new InvalidImageContentException("Box content is too large to buffer."); + } + + int bufferLength = (int)length; + IMemoryOwner buffer = this.configuration.MemoryAllocator.Allocate(bufferLength); int bytesRead = stream.Read(buffer.GetSpan()); - if (bytesRead != length) + if (bytesRead != bufferLength) { - throw new InvalidImageContentException("Stream length not sufficient for box content"); + throw new InvalidImageContentException("Stream length is not sufficient for box content."); } return buffer; @@ -781,9 +857,9 @@ internal sealed class HeifDecoderCore : ImageDecoderCore private static void EnsureBoxInsideParent(long boxLength, long parentLength) { - if (boxLength > parentLength) + if (boxLength < 0 || parentLength < 0 || boxLength > parentLength) { - throw new ImageFormatException("Box size beyond boundary"); + throw new InvalidImageContentException("Box size extends beyond its parent boundary."); } } diff --git a/tests/ImageSharp.Tests/Formats/Heif/HeifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Heif/HeifDecoderTests.cs index 7366fde31e..4b9b2ebc84 100644 --- a/tests/ImageSharp.Tests/Formats/Heif/HeifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Heif/HeifDecoderTests.cs @@ -149,6 +149,145 @@ public class HeifDecoderTests Assert.Throws(() => HeifDecoder.Instance.Identify(DecoderOptions.Default, stream)); } + [Fact] + public void IdentifyAcceptsExtendedSizeTopLevelBox() + { + byte[] data = CreateEncodedContainer(); + byte[] box = new byte[16]; + BinaryPrimitives.WriteUInt32BigEndian(box, 1); + BinaryPrimitives.WriteUInt32BigEndian(box.AsSpan(4), UnknownBoxType); + BinaryPrimitives.WriteUInt64BigEndian(box.AsSpan(8), (ulong)box.Length); + data = InsertBytes(data, data.Length, box); + + ImageInfo imageInfo = Image.Identify(data); + + Assert.Equal(new Size(2, 3), imageInfo.Size); + } + + [Fact] + public void IdentifyAcceptsUuidTopLevelBox() + { + byte[] data = CreateEncodedContainer(); + byte[] box = new byte[24]; + BinaryPrimitives.WriteUInt32BigEndian(box, (uint)box.Length); + BinaryPrimitives.WriteUInt32BigEndian(box.AsSpan(4), (uint)Heif4CharCode.Uuid); + data = InsertBytes(data, data.Length, box); + + ImageInfo imageInfo = Image.Identify(data); + + Assert.Equal(new Size(2, 3), imageInfo.Size); + } + + [Fact] + public void IdentifyAcceptsSizeZeroTopLevelBox() + { + byte[] data = CreateEncodedContainer(); + byte[] box = CreateUnknownBox(); + BinaryPrimitives.WriteUInt32BigEndian(box, 0); + data = InsertBytes(data, data.Length, box); + + ImageInfo imageInfo = Image.Identify(data); + + Assert.Equal(new Size(2, 3), imageInfo.Size); + } + + [Fact] + public void IdentifyAcceptsExtendedSizeItemInfoEntry() + { + byte[] data = CreateEncodedContainer(); + int metaOffset = FindBoxOffset(data, Heif4CharCode.Meta, 0, data.Length); + int metaSize = (int)BinaryPrimitives.ReadUInt32BigEndian(data.AsSpan(metaOffset)); + int iinfOffset = FindBoxOffset(data, Heif4CharCode.Iinf, metaOffset + 12, metaSize - 12); + int infeOffset = iinfOffset + 14; + uint infeSize = BinaryPrimitives.ReadUInt32BigEndian(data.AsSpan(infeOffset)); + data = InsertBytes(data, infeOffset + 8, new byte[8]); + BinaryPrimitives.WriteUInt32BigEndian(data.AsSpan(infeOffset), 1); + BinaryPrimitives.WriteUInt64BigEndian(data.AsSpan(infeOffset + 8), infeSize + 8); + IncrementBoxSize(data, metaOffset, 8); + IncrementBoxSize(data, iinfOffset, 8); + + ImageInfo imageInfo = Image.Identify(data); + + Assert.Equal(new Size(2, 3), imageInfo.Size); + } + + [Fact] + public void IdentifyRejectsSizeZeroMetadataChild() + { + byte[] data = CreateEncodedContainer(); + int metaOffset = FindBoxOffset(data, Heif4CharCode.Meta, 0, data.Length); + int metaSize = (int)BinaryPrimitives.ReadUInt32BigEndian(data.AsSpan(metaOffset)); + byte[] box = CreateUnknownBox(); + BinaryPrimitives.WriteUInt32BigEndian(box, 0); + data = InsertBytes(data, metaOffset + metaSize, box); + IncrementBoxSize(data, metaOffset, box.Length); + + Assert.Throws(() => Image.Identify(data)); + } + + [Fact] + public void IdentifyRejectsMetadataChildBeyondParent() + { + byte[] data = CreateEncodedContainer(); + int metaOffset = FindBoxOffset(data, Heif4CharCode.Meta, 0, data.Length); + int metaSize = (int)BinaryPrimitives.ReadUInt32BigEndian(data.AsSpan(metaOffset)); + byte[] box = CreateUnknownBox(); + BinaryPrimitives.WriteUInt32BigEndian(box, 16); + data = InsertBytes(data, metaOffset + metaSize, box); + IncrementBoxSize(data, metaOffset, box.Length); + + Assert.Throws(() => Image.Identify(data)); + } + + [Fact] + public void IdentifyRejectsItemInfoEntryBeyondParent() + { + byte[] data = CreateEncodedContainer(); + int metaOffset = FindBoxOffset(data, Heif4CharCode.Meta, 0, data.Length); + int metaSize = (int)BinaryPrimitives.ReadUInt32BigEndian(data.AsSpan(metaOffset)); + int iinfOffset = FindBoxOffset(data, Heif4CharCode.Iinf, metaOffset + 12, metaSize - 12); + uint iinfSize = BinaryPrimitives.ReadUInt32BigEndian(data.AsSpan(iinfOffset)); + int infeOffset = iinfOffset + 14; + BinaryPrimitives.WriteUInt32BigEndian(data.AsSpan(infeOffset), iinfSize); + + Assert.Throws(() => Image.Identify(data)); + } + + [Fact] + public void IdentifyRejectsBoxSmallerThanHeader() + { + byte[] data = CreateEncodedContainer(); + byte[] box = CreateUnknownBox(); + BinaryPrimitives.WriteUInt32BigEndian(box, 4); + data = InsertBytes(data, data.Length, box); + + Assert.Throws(() => Image.Identify(data)); + } + + [Fact] + public void IdentifyRejectsTruncatedExtendedSizeHeader() + { + byte[] data = CreateEncodedContainer(); + byte[] box = new byte[12]; + BinaryPrimitives.WriteUInt32BigEndian(box, 1); + BinaryPrimitives.WriteUInt32BigEndian(box.AsSpan(4), UnknownBoxType); + data = InsertBytes(data, data.Length, box); + + Assert.Throws(() => Image.Identify(data)); + } + + [Fact] + public void IdentifyRejectsTruncatedUuidHeader() + { + byte[] data = CreateEncodedContainer(); + byte[] box = new byte[16]; + BinaryPrimitives.WriteUInt32BigEndian(box, 24); + BinaryPrimitives.WriteUInt32BigEndian(box.AsSpan(4), (uint)Heif4CharCode.Uuid); + data = InsertBytes(data, data.Length, box); + + Assert.Throws(() => Image.Identify(data)); + } + private static byte[] CreateEncodedContainer() { using Image image = new(2, 3);