Skip to content

Make the WebP decoder tolerate reserved VP8X bits, trailing data and odd-length metadata chunks - #3233

Open
jafin wants to merge 1 commit into
SixLabors:mainfrom
jafin:jf/webp-decoder-container-robustness
Open

jafin wants to merge 1 commit into
SixLabors:mainfrom
jafin:jf/webp-decoder-container-robustness

Conversation

@jafin

@jafin jafin commented Oct 17, 2026 •

Copy link
Copy Markdown

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

This makes the WebP decoder accept files that libwebp and browsers open, and makes it read odd-length metadata chunks exactly as they were written.

Reserved VP8X flag bits were rejected

The decoder threw ImageFormatException ("first two bits of the VP8X header are expected to be zero") when either of the top two reserved bits of the VP8X flags byte was set. The container specification says readers must ignore reserved fields. The reserved bytes after the flags byte were already ignored. I removed the check, so a file with non-zero reserved bits now decodes to the same image as the unmodified file.

Bytes after the RIFF payload broke decoding

The chunk loops in ReadVp8Info and ParseOptionalChunks ran to the end of the stream instead of the end of the RIFF payload. For an extended (VP8X) file, 1 to 3 trailing bytes threw "could not read chunk type". Longer trailing data was parsed as more chunks: a valid image chunk there would replace the image, and an ImageFormatException thrown this way also left the image bit reader's buffers undisposed. One of the existing test files, Issue2925.webp, has 80 KB of zero bytes after its RIFF payload.

The decoder now records where the RIFF payload ends and reads chunks only up to that point, as libwebp does. If the declared size is larger than the stream (a truncated file), it reads up to the end of the stream instead. The loops also stop when fewer bytes than a chunk header remain. Unknown chunks were skipped with stream.Skip((int)chunkSize), so a size above int.MaxValue became a negative skip and the decoder then read the chunk's payload as chunk headers. These skips are now clamped to the end of the RIFF payload.

Truncated container fields threw ImageFormatException

When the stream ended inside a chunk type, a chunk size or a 24-bit VP8X/ANMF field, the decoder threw a plain ImageFormatException. It now throws InvalidImageContentException, which derives from ImageFormatException. A VP8L chunk cut off before its 5-byte header was read from zero-filled memory and reported as "Invalid VP8L signature". It is now reported as missing data.

Truncation inside the compressed image data itself is unchanged: the bitstream decoders still report it as they do today. The existing WebpDecoder_ThrowImageFormatException_OnInvalidImages test pins that behaviour for a truncated lossless file.

Odd-length metadata chunks kept their padding byte

RIFF follows an odd-sized payload with one padding byte. The metadata readers used the padded size as the payload length, so an odd-length ICCP payload came back with an extra trailing 0x00, and saving the image wrote that byte back as part of the profile. EXIF and XMP chunks had the same defect. It was hidden in practice because the EXIF profile is re-serialised and XMP trailing NULs are trimmed. ReadIccProfile, ReadExifProfile and ReadXmpProfile now read the stored payload size, and ReadMetadataChunk skips the padding byte after the payload. This keeps the next chunk aligned and does not change even-length chunks or SkipMetadata.

Test fixture correction

Decode_WithMetadataChunkLargerThanRemainingData_ThrowsInStrictMode used a fixture with a RIFF size of 0. With the RIFF bound, that fixture would fail early with "Missing WebP image data" and never reach the metadata path the test is for. I set the fixture's RIFF size to its real payload length (34), so the test exercises the oversized-metadata path again. Its assertions are unchanged.

Already handled on main

EXIF and XMP chunks that declare sizes up to 0xFFFFFFFF are already bounded by the remaining stream before any allocation. They are skipped, or rejected in strict mode, and an oversized ICCP chunk throws InvalidImageContentException. The existing Decode_TruncatedTrailingMetadata_RespectsOptions and Decode_WithOversizedMetadataChunk_* tests cover this, so I added no tests for it.

Tests

All new tests build their inputs in memory by encoding a small image and changing the bytes. No new image files are added.

  • Decode_Vp8XWithNonZeroReservedFields_MatchesOriginal: reserved flag bits and reserved bytes.
  • Decode_WithBytesAfterRiffPayload_MatchesOriginal: 1 to 64 random trailing bytes on simple lossy, simple lossless and VP8X files with alpha, EXIF, EXIF + XMP and ICC.
  • Decode_WithImageChunkAfterRiffPayload_IgnoresIt: a complete VP8L chunk after the payload.
  • Decode_UnknownChunkLargerThanRiffPayload_MatchesOriginal
  • Decode_TruncatedContainer_DecodesOrThrowsInvalidImageContentException: every truncation length, for both Load and Identify.
  • Decode_OddLengthMetadataChunks_ReadsPayloadsWithoutPaddingByte: odd ICCP, EXIF and XMP chunks, with and without SkipMetadata.

All of these fail on main, except the variants that already worked there. They pass with this change. The decoder tests run under [ValidateDisposedMemoryAllocations]. All WebP tests pass in Release on net10.0.

Fixes #3245

…odd-length metadata chunks

Readers must ignore the reserved VP8X flag bits, so they are no longer
validated. Chunks are now read only up to the end of the RIFF payload,
so bytes after it are ignored rather than parsed as more chunks, and
unknown chunks larger than the payload end the parsing instead of
becoming a negative skip. A stream that ends inside a chunk header,
a 24 bit field or the VP8L header now throws
InvalidImageContentException.

Metadata chunks with an odd payload length are read without the RIFF
padding byte, which is skipped before the next chunk.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WebP decoder rejects files with reserved VP8X bits set or bytes after the RIFF payload, and keeps the padding byte of odd-length metadata chunks

1 participant