Harden AV1 OBU framing

HEIF_IMPLEMENTATION_PLAN.md

@@ -496,6 +496,7 @@ Exit gate:
 Implement and verify in dependency order:
 
 - [ ] OBU framing, sequence headers, frame headers, tile groups, byte alignment, and trailing bits.
+  - [x] Isolate every declared OBU payload with a zero-copy bounded span reader, advance ignored metadata and reserved units without parsing their bytes as headers, validate padding and trailing bytes, enforce primary/redundant/combined frame-header order, and reject lengths that cross the containing image-item boundary. Focused malformed-input tests and independent 8/10/12-bit AVIF reconstruction pass in Release.
 - [ ] One coherent decoder lifecycle that retains parsed frame and tile state and disposes all buffers deterministically.
 - [ ] Tile partitioning, mode information, segmentation, delta quantization, transform-size selection, coefficient token decode, inverse quantization, and inverse transforms.
   - [x] Match libaom's depth-first traversal and frame-edge behavior for all ten AV1 partition types. Independent 8/10/12-bit streams collectively select every terminal partition shape and contain nested block geometry that requires recursive `Split` traversal; their complete native planes remain byte-exact under normal hardware dispatch and the scalar fallback.

src/ImageSharp/Formats/Heif/Av1/Av1BitStreamReader.cs

@@ -87,18 +87,20 @@ internal ref struct Av1BitStreamReader
 
         ulong value = 0;
         length = 0;
-        for (int i = 0; i < 56; i += 7)
+        for (int shift = 0; shift < 56; shift += 7)
         {
             uint leb128Byte = this.ReadLiteral(8);
-            value |= (leb128Byte & 0x7FUL) << i;
+            value |= (leb128Byte & 0x7FUL) << shift;
             length++;
             if ((leb128Byte & 0x80U) == 0)
             {
-                break;
+                return value;
             }
         }
 
-        return value;
+        // AV1 limits unsigned LEB128 fields to eight bytes. A continuation bit in the eighth byte does not describe
+        // another value byte; accepting it would move the following OBU header into the declared size field.
+        throw new InvalidImageContentException("The AV1 LEB128 value is not terminated within eight bytes.");
     }
 
     /// <summary>
@@ -204,11 +206,24 @@ internal ref struct Av1BitStreamReader
     /// <param name="tileDataSize">The tile payload length in bytes.</param>
     /// <returns>The tile payload span.</returns>
     public Span<byte> GetSymbolReader(int tileDataSize)
+        => this.ReadBytes(tileDataSize);
+
+    /// <summary>
+    /// Gets the next byte-aligned portion of the encoded data and advances past it.
+    /// </summary>
+    /// <param name="byteCount">The number of bytes to read.</param>
+    /// <returns>The requested bytes.</returns>
+    public Span<byte> ReadBytes(int byteCount)
     {
-        DebugGuard.IsTrue(Av1Math.Modulus8(this.BitPosition) == 0, "Symbol reading needs to start on byte boundary.");
+        DebugGuard.IsTrue(Av1Math.Modulus8(this.BitPosition) == 0, "Byte spans must start on a byte boundary.");
-        int bytesRead = Av1Math.DivideBy8Floor(this.BitPosition);
+        int byteOffset = Av1Math.DivideBy8Floor(this.BitPosition);
-        Span<byte> span = this.data.Slice(bytesRead, tileDataSize);
+        if ((uint)byteOffset > (uint)this.data.Length || (uint)byteCount > (uint)(this.data.Length - byteOffset))
-        this.Skip(tileDataSize << 3);
+        {
-        return span;
+            throw new InvalidImageContentException("The AV1 payload exceeds its declared data boundary.");
+        }
+
+        Span<byte> payload = this.data.Slice(byteOffset, byteCount);
+        this.Skip(byteCount << 3);
+        return payload;
     }
 }

src/ImageSharp/Formats/Heif/Av1/OpenBitstreamUnit/ObuHeader.cs

@@ -9,7 +9,7 @@ namespace SixLabors.ImageSharp.Formats.Heif.Av1.OpenBitstreamUnit;
 internal class ObuHeader
 {
     /// <summary>
-    /// Gets or sets the number of bytes occupied by the OBU header and its size field.
+    /// Gets or sets the number of bytes occupied by the fixed OBU header and optional extension.
     /// </summary>
     public int Size { get; set; }
 

src/ImageSharp/Formats/Heif/Av1/OpenBitstreamUnit/ObuReader.cs

@@ -35,101 +35,219 @@ internal class ObuReader
     /// <param name="isAnnexB">A value indicating whether each OBU is prefixed by an Annex B length field.</param>
     public void ReadAll(ref Av1BitStreamReader reader, int dataSize, Func<IAv1TileReader> creator, bool isAnnexB = false)
     {
-        bool seenFrameHeader = false;
+        try
-        bool frameDecodingFinished = false;
-        while (!frameDecodingFinished)
         {
-            int lengthSize = 0;
+            int availableByteCount = reader.Length - Av1Math.DivideBy8Floor(reader.BitPosition);
-            int payloadSize = 0;
+            if ((reader.BitPosition & 0x7) != 0 || (uint)dataSize > (uint)availableByteCount)
-            if (isAnnexB)
             {
-                ReadObuSize(ref reader, out payloadSize, out lengthSize);
+                throw new InvalidImageContentException("The AV1 OBU data boundary is invalid.");
             }
 
-            ObuHeader header = ReadObuHeaderSize(ref reader, out lengthSize);
+            bool seenFrameHeader = false;
-            if (isAnnexB)
+            bool frameDecodingFinished = false;
+            Span<byte> primaryFrameHeaderPayload = default;
+            while (dataSize > 0 && !frameDecodingFinished)
             {
-                header.PayloadSize -= header.Size;
+                int annexObuSize = 0;
-                dataSize -= lengthSize;
+                if (isAnnexB)
-                lengthSize = 0;
+                {
-            }
+                    ReadObuSize(ref reader, out annexObuSize, out int annexLengthSize);
-
+                    if (annexLengthSize > dataSize || annexObuSize < 1)
-            payloadSize = header.PayloadSize;
-            dataSize -= header.Size + lengthSize;
-            if (isAnnexB && dataSize < payloadSize)
-            {
-                throw new InvalidImageContentException("Corrupt frame");
-            }
-
-            switch (header.Type)
-            {
-                case ObuType.SequenceHeader:
-                    this.SequenceHeader = new();
-                    ReadSequenceHeader(ref reader, this.SequenceHeader);
-                    break;
-                case ObuType.FrameHeader:
-                case ObuType.RedundantFrameHeader:
-                case ObuType.Frame:
-                    if (header.Type != ObuType.Frame)
-                    {
-                        // Nothing to do here.
-                    }
-                    else if (header.Type != ObuType.FrameHeader)
                     {
-                        Guard.IsFalse(seenFrameHeader, nameof(seenFrameHeader), "Frame header expected");
+                        throw new InvalidImageContentException("The Annex B AV1 OBU length is invalid.");
                     }
-                    else
+
+                    dataSize -= annexLengthSize;
+                    if (annexObuSize > dataSize)
                     {
-                        Guard.IsTrue(seenFrameHeader, nameof(seenFrameHeader), "Already decoded a frame header");
+                        throw new InvalidImageContentException("The Annex B AV1 OBU exceeds its temporal-unit boundary.");
                     }
+                }
+                else if (dataSize < 1)
+                {
+                    throw new InvalidImageContentException("The AV1 OBU header is truncated.");
+                }
+
+                int obuStartBitPosition = reader.BitPosition;
+                ObuHeader header = ReadObuHeaderSize(ref reader, out _);
+                if (!isAnnexB && !header.HasSize)
+                {
+                    // AV1 section 5 requires every low-overhead OBU to carry its own payload size. Only Annex B may
+                    // derive the payload length from the outer obu_length field.
+                    throw new InvalidImageContentException("A low-overhead AV1 OBU is missing its payload-size field.");
+                }
+
+                int headerAndLengthSize = (reader.BitPosition - obuStartBitPosition) >> 3;
+                int boundedObuSize = isAnnexB ? annexObuSize : dataSize;
+                if (headerAndLengthSize > boundedObuSize)
+                {
+                    throw new InvalidImageContentException("The AV1 OBU header exceeds its declared boundary.");
+                }
+
+                int payloadSize = header.HasSize ? header.PayloadSize : boundedObuSize - headerAndLengthSize;
+                if ((uint)payloadSize > (uint)(boundedObuSize - headerAndLengthSize))
+                {
+                    throw new InvalidImageContentException("The AV1 OBU payload exceeds its declared boundary.");
+                }
+
+                int completeObuSize = headerAndLengthSize + payloadSize;
+                if (isAnnexB && completeObuSize != annexObuSize)
+                {
+                    throw new InvalidImageContentException("The nested and Annex B AV1 OBU lengths do not match.");
+                }
+
+                dataSize -= isAnnexB ? annexObuSize : completeObuSize;
+                header.PayloadSize = payloadSize;
+
+                // A dedicated payload reader prevents malformed syntax from consuming the following OBU. The parent
+                // advances once here, so ignored metadata, padding, and reserved OBUs are skipped without copying.
+                Span<byte> obuPayload = reader.ReadBytes(payloadSize);
+                Av1BitStreamReader payloadReader = new(obuPayload);
+                int decodedPayloadSize;
+
+                switch (header.Type)
+                {
+                    case ObuType.SequenceHeader:
+                        this.SequenceHeader = new();
+                        ReadSequenceHeader(ref payloadReader, this.SequenceHeader);
+                        decodedPayloadSize = Av1Math.DivideBy8Floor(payloadReader.BitPosition);
+                        break;
+                    case ObuType.FrameHeader:
+                        if (this.SequenceHeader is null)
+                        {
+                            throw new InvalidImageContentException("An AV1 frame header appears before its sequence header.");
+                        }
+
+                        if (seenFrameHeader)
+                        {
+                            throw new InvalidImageContentException("An AV1 frame contains more than one primary frame header.");
+                        }
 
-                    if (!seenFrameHeader)
-                    {
                         seenFrameHeader = true;
                         this.FrameHeader = new();
-                        this.ReadFrameHeader(ref reader, header, header.Type != ObuType.Frame);
+                        this.ReadFrameHeader(ref payloadReader, header, trailingBit: true);
-                    }
+                        decodedPayloadSize = Av1Math.DivideBy8Floor(payloadReader.BitPosition);
+                        primaryFrameHeaderPayload = obuPayload[..decodedPayloadSize];
+                        break;
+                    case ObuType.RedundantFrameHeader:
+                        if (!seenFrameHeader)
+                        {
+                            throw new InvalidImageContentException("A redundant AV1 frame header appears before its primary frame header.");
+                        }
 
-                    if (header.Type != ObuType.Frame)
+                        if (primaryFrameHeaderPayload.Length > obuPayload.Length
-                    {
+                            || !obuPayload[..primaryFrameHeaderPayload.Length].SequenceEqual(primaryFrameHeaderPayload))
-                        break; // For OBU_TILE_GROUP comes under OBU_FRAME
+                        {
-                    }
+                            throw new InvalidImageContentException("The redundant AV1 frame header does not match its primary header.");
+                        }
 
-                    goto TILE_GROUP;
+                        // The primary header already owns the decoded frame state. Matching its encoded bytes avoids
-                case ObuType.TileGroup:
+                        // parsing the same adaptive frame-header syntax twice, as in libaom's decoder.
-                    TILE_GROUP:
+                        decodedPayloadSize = primaryFrameHeaderPayload.Length;
-                    if (!seenFrameHeader)
+                        break;
-                    {
+                    case ObuType.Frame:
-                        throw new InvalidImageContentException("Corrupt frame");
+                        if (this.SequenceHeader is null)
-                    }
+                        {
+                            throw new InvalidImageContentException("An AV1 frame appears before its sequence header.");
+                        }
 
-                    this.decoder ??= creator();
+                        if (seenFrameHeader)
+                        {
+                            throw new InvalidImageContentException("A combined AV1 frame OBU follows a separate frame header.");
+                        }
 
-                    // A combined frame OBU reaches this label after its frame-header portion has
+                        seenFrameHeader = true;
-                    // been consumed, leaving the same tile-group syntax as a standalone tile OBU.
+                        this.FrameHeader = new();
-                    this.ReadTileGroup(ref reader, this.decoder, header, out frameDecodingFinished);
+                        this.ReadFrameHeader(ref payloadReader, header, trailingBit: false);
-                    if (frameDecodingFinished)
+                        primaryFrameHeaderPayload = obuPayload[..Av1Math.DivideBy8Floor(payloadReader.BitPosition)];
-                    {
+                        goto TILE_GROUP;
+                    case ObuType.TileGroup:
+                        TILE_GROUP:
+                        if (!seenFrameHeader)
+                        {
+                            throw new InvalidImageContentException("An AV1 tile group appears before its frame header.");
+                        }
+
+                        this.decoder ??= creator();
+
+                        // A combined frame OBU reaches this label after its frame-header portion has
+                        // been consumed, leaving the same tile-group syntax as a standalone tile OBU.
+                        this.ReadTileGroup(ref payloadReader, this.decoder, header, out frameDecodingFinished);
+                        decodedPayloadSize = Av1Math.DivideBy8Floor(payloadReader.BitPosition);
+                        if (frameDecodingFinished)
+                        {
+                            seenFrameHeader = false;
+                        }
+
+                        break;
+                    case ObuType.TemporalDelimiter:
+                        // AV1 section 5.6 defines no delimiter syntax. The common post-switch validation still permits
+                        // zero bytes between the empty syntax and the declared payload boundary, matching libaom.
                         seenFrameHeader = false;
-                    }
+                        decodedPayloadSize = 0;
+                        break;
+                    case ObuType.Padding:
+                        int lastNonzeroIndex = obuPayload.Length - 1;
+                        while (lastNonzeroIndex >= 0 && obuPayload[lastNonzeroIndex] == 0)
+                        {
+                            lastNonzeroIndex--;
+                        }
 
-                    break;
+                        // AV1 padding contains only its trailing one bit and optional zero bytes. A header-only
-                case ObuType.TemporalDelimiter:
+                        // padding OBU is also valid, so the empty payload bypasses this final-byte check.
-                    // 5.6. Temporal delimiter obu syntax.
+                        if (lastNonzeroIndex >= 0 && obuPayload[lastNonzeroIndex] != 0x80)
-                    seenFrameHeader = false;
+                        {
-                    break;
+                            throw new InvalidImageContentException("The AV1 padding OBU has invalid trailing bits.");
-                default:
+                        }
-                    // Ignore unknown OBU types.
-                    // throw new InvalidImageContentException($"Unknown OBU header found: {header.Type.ToString()}");
-                    break;
-            }
 
-            dataSize -= payloadSize;
+                        if (obuPayload.Length > 0 && lastNonzeroIndex < 0)
-            if (dataSize <= 0)
+                        {
-            {
+                            throw new InvalidImageContentException("The AV1 padding OBU is missing its trailing one bit.");
-                frameDecodingFinished = true;
+                        }
+
+                        decodedPayloadSize = payloadSize;
+                        break;
+                    default:
+                        // Metadata, tile-list, and reserved OBUs do not contribute to this still-image reconstruction
+                        // pass. Their declared payload has already been skipped by the parent reader. libaom rejects a
+                        // nonempty unrecognized payload that contains only zeros because it has no trailing one bit.
+                        if (payloadSize > 0)
+                        {
+                            int ignoredLastNonzeroIndex = payloadSize - 1;
+                            while (ignoredLastNonzeroIndex >= 0 && obuPayload[ignoredLastNonzeroIndex] == 0)
+                            {
+                                ignoredLastNonzeroIndex--;
+                            }
+
+                            if (ignoredLastNonzeroIndex < 0)
+                            {
+                                throw new InvalidImageContentException("The ignored AV1 OBU is missing its trailing one bit.");
+                            }
+                        }
+
+                        decodedPayloadSize = payloadSize;
+                        break;
+                }
+
+                // Parsed syntax may be followed only by zero bytes within its declared OBU payload. Ignored metadata
+                // and reserved OBUs set decodedPayloadSize to the full payload because their syntax is not consumed here.
+                for (int i = decodedPayloadSize; i < obuPayload.Length; i++)
+                {
+                    if (obuPayload[i] != 0)
+                    {
+                        throw new InvalidImageContentException("The AV1 OBU contains nonzero data after its decoded syntax.");
+                    }
+                }
             }
         }
+        catch (IndexOutOfRangeException exception)
+        {
+            throw new InvalidImageContentException("The AV1 OBU syntax exceeds its payload boundary.", exception);
+        }
+        catch (ArgumentOutOfRangeException exception)
+        {
+            throw new InvalidImageContentException("The AV1 OBU syntax exceeds its payload boundary.", exception);
+        }
     }
 
     /// <summary>
@@ -182,9 +300,9 @@ internal class ObuReader
     private static void ReadObuSize(ref Av1BitStreamReader reader, out int obuSize, out int lengthSize)
     {
         ulong rawSize = reader.ReadLittleEndianBytes128(out lengthSize);
-        if (rawSize > uint.MaxValue)
+        if (rawSize > int.MaxValue)
         {
-            throw new ImageFormatException("OBU block too large.");
+            throw new InvalidImageContentException("The AV1 OBU size exceeds the supported image payload limit.");
         }
 
         obuSize = (int)rawSize;

tests/ImageSharp.Tests/Formats/Heif/Av1/ObuFrameHeaderTests.cs

@@ -12,8 +12,6 @@ public class ObuFrameHeaderTests
     private static readonly byte[] DefaultSequenceHeaderBitStream =
         [0x0a, 0x06, 0b001_1_1_000, 0b00_1000_01, 0b11_110101, 0b001_11101, 0b111_1_1_1_0_1, 0b1_0_0_1_1_1_10];
 
-    private static readonly byte[] KeyFrameHeaderBitStream = [0x32, 0x06, 0x10, 0x00];
-
     // Bits  Syntax element                  Value
     // 1     obu_forbidden_bit               0
     // 4     obu_type                        2 (OBU_TEMPORAL_DELIMITER)
@@ -133,16 +131,17 @@ public class ObuFrameHeaderTests
     }
 
     [Fact]
-    public void ReadHeaderWithoutSizeField()
+    public void ReadAnnexBHeaderWithoutSizeField()
     {
         // Arrange
-        byte[] bitStream = [0x10];
+        // Annex B's outer obu_length is one byte and covers the size-less temporal-delimiter header.
+        byte[] bitStream = [0x01, 0x10];
         Av1BitStreamReader reader = new(bitStream);
         ObuReader obuReader = new();
         IAv1TileReader tileDecoder = new Av1TileDecoderStub();
 
         // Act
-        obuReader.ReadAll(ref reader, bitStream.Length, () => tileDecoder);
+        obuReader.ReadAll(ref reader, bitStream.Length, () => tileDecoder, isAnnexB: true);
 
         // Assert
         Assert.Null(obuReader.SequenceHeader);
@@ -168,6 +167,54 @@ public class ObuFrameHeaderTests
         Assert.Equal(ObuPrettyPrint.PrettyPrintProperties(expected), ObuPrettyPrint.PrettyPrintProperties(obuReader.SequenceHeader));
     }
 
+    /// <summary>
+    /// Verifies that ignored OBU payloads are bounded and skipped before parsing the following sequence header.
+    /// </summary>
+    [Fact]
+    public void ReadIgnoredObusSkipsEachDeclaredPayload()
+    {
+        // 0x7A identifies padding and 0x4A identifies reserved OBU type 9. Both carry explicit sizes so their payload
+        // bytes must never be interpreted as another OBU header.
+        byte[] bitStream =
+        [
+            0x7A, 0x02, 0x80, 0x00,
+            0x4A, 0x01, 0x80,
+            .. DefaultSequenceHeaderBitStream
+        ];
+
+        Av1BitStreamReader reader = new(bitStream);
+        ObuReader obuReader = new();
+        IAv1TileReader tileDecoder = new Av1TileDecoderStub();
+
+        obuReader.ReadAll(ref reader, bitStream.Length, () => tileDecoder);
+
+        Assert.NotNull(obuReader.SequenceHeader);
+        Assert.Equal(bitStream.Length * 8, reader.BitPosition);
+    }
+
+    /// <summary>
+    /// Verifies that invalid OBU boundaries, size fields, and trailing bytes are rejected.
+    /// </summary>
+    /// <param name="bitStream">The malformed OBU stream.</param>
+    [Theory]
+    [InlineData(new byte[] { 0x7A, 0x02, 0x11 })]
+    [InlineData(new byte[] { 0x7A, 0x01, 0x00 })]
+    [InlineData(new byte[] { 0x12, 0x01, 0x01 })]
+    [InlineData(new byte[] { 0x10 })]
+    [InlineData(new byte[] { 0x7A, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80 })]
+    public void ReadInvalidObuBoundaryThrows(byte[] bitStream)
+        => Assert.Throws<InvalidImageContentException>(() => ReadObuStream(bitStream));
+
+    /// <summary>
+    /// Verifies that an empty temporal delimiter may occupy a payload containing only zero padding bytes.
+    /// </summary>
+    [Fact]
+    public void ReadTemporalDelimiterAllowsZeroPayloadPadding()
+    {
+        byte[] bitStream = [0x12, 0x02, 0x00, 0x00];
+        ReadObuStream(bitStream);
+    }
+
     [Fact]
     public void WriteTemporalDelimiter()
     {
@@ -201,6 +248,9 @@ public class ObuFrameHeaderTests
         Assert.Equal(DefaultSequenceHeaderBitStream, actual);
     }
 
+    /// <summary>
+    /// Verifies that the combined frame OBU declares exactly the payload bytes emitted by the writer.
+    /// </summary>
     [Fact]
     public void WriteFrameHeader()
     {
@@ -215,12 +265,25 @@ public class ObuFrameHeaderTests
 
         // Act
         obuWriter.WriteAll(Configuration.Default, stream, sequenceInput, frameInput, tileStub);
-        byte[] buffer = stream.GetBuffer();
+        byte[] bitStream = stream.ToArray();
 
         // Assert
-        // Skip over Temporal Delimiter and Sequence header.
+        int frameOffset = DefaultTemporalDelimiterBitStream.Length + DefaultSequenceHeaderBitStream.Length;
-        byte[] actual = buffer.AsSpan().Slice(DefaultTemporalDelimiterBitStream.Length + DefaultSequenceHeaderBitStream.Length, KeyFrameHeaderBitStream.Length).ToArray();
+        Span<byte> frameObu = bitStream.AsSpan(frameOffset);
-        Assert.Equal(KeyFrameHeaderBitStream, actual);
+        byte expectedHeader = (byte)(((byte)ObuType.Frame << 3) | 0x02);
+        Assert.Equal(expectedHeader, frameObu[0]);
+
+        Av1BitStreamReader sizeReader = new(frameObu[1..]);
+        ulong declaredPayloadSize = sizeReader.ReadLittleEndianBytes128(out int encodedSizeLength);
+        Assert.Equal(frameObu.Length - 1 - encodedSizeLength, (int)declaredPayloadSize);
+
+        Av1BitStreamReader reader = new(bitStream);
+        ObuReader obuReader = new();
+        obuReader.ReadAll(ref reader, bitStream.Length, () => new Av1TileDecoderStub());
+
+        Assert.NotNull(obuReader.SequenceHeader);
+        Assert.NotNull(obuReader.FrameHeader);
+        Assert.Equal(bitStream.Length * 8, reader.BitPosition);
     }
 
     private static ObuSequenceHeader GetDefaultSequenceHeader()
@@ -290,6 +353,19 @@ public class ObuFrameHeaderTests
                 AreFilmGrainingParametersPresent = true,
             };
 
+    /// <summary>
+    /// Reads one complete OBU stream for malformed-input assertions that cannot capture a ref-struct reader.
+    /// </summary>
+    /// <param name="bitStream">The complete encoded OBU stream.</param>
+    private static void ReadObuStream(byte[] bitStream)
+    {
+        Av1BitStreamReader reader = new(bitStream);
+        ObuReader obuReader = new();
+        IAv1TileReader tileDecoder = new Av1TileDecoderStub();
+
+        obuReader.ReadAll(ref reader, bitStream.Length, () => tileDecoder);
+    }
+
     private static ObuFrameHeader GetKeyFrameHeader()
         => new()
         {