Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jafin wants to merge 1 commit into
base: main
Choose a base branch
from
+394 −47
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 46 additions & 35 deletions src/ImageSharp/Formats/Webp/WebpChunkParsingUtils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,14 @@ public static WebpImageInfo ReadVp8LHeader(MemoryAllocator memoryAllocator, Buff
// VP8 data size.
uint imageDataSize = ReadChunkSize(stream, buffer);

// The header is a one byte signature followed by 32 bits holding the width, height, alpha flag and version.
// The bit reader zero-fills data missing from a truncated stream, so check that the header itself is present
// rather than reporting the zeros as an invalid signature.
if (stream.RemainingBytes < 5)
{
WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the VP8L header");
}

Vp8LBitReader bitReader = new(stream, imageDataSize, memoryAllocator);

// One byte signature, should be 0x2f.
Expand Down Expand Up @@ -209,14 +217,10 @@ public static WebpImageInfo ReadVp8XHeader(BufferedReadStream stream, Span<byte>
uint fileSize = ReadChunkSize(stream, buffer);

// The first byte contains information about the image features used.
// Its two most significant bits and its least significant bit are reserved. Writers set them to zero,
// but the container specification requires readers to ignore them, so they are not validated.
byte imageFeatures = (byte)stream.ReadByte();

// The first two bit of it are reserved and should be 0.
if (imageFeatures >> 6 != 0)
{
WebpThrowHelper.ThrowImageFormatException("first two bits of the VP8X header are expected to be zero");
}

// If bit 3 is set, a ICC Profile Chunk should be present.
features.IccProfile = (imageFeatures & (1 << 5)) != 0;

Expand Down Expand Up @@ -259,8 +263,8 @@ public static WebpImageInfo ReadVp8XHeader(BufferedReadStream stream, Span<byte>
/// <param name="stream">The stream to read from.</param>
/// <param name="buffer">The buffer to store the read data into.</param>
/// <returns>A unsigned 24 bit integer.</returns>
/// <exception cref="ImageFormatException">
/// Thrown if the input stream is not valid.
/// <exception cref="InvalidImageContentException">
/// Thrown if the stream ends before the integer.
/// </exception>
public static uint ReadUInt24LittleEndian(BufferedReadStream stream, Span<byte> buffer)
{
Expand All @@ -270,7 +274,7 @@ public static uint ReadUInt24LittleEndian(BufferedReadStream stream, Span<byte>
return BinaryPrimitives.ReadUInt32LittleEndian(buffer);
}

throw new ImageFormatException("Invalid Webp data, could not read unsigned 24 bit integer.");
throw new InvalidImageContentException("Invalid Webp data, could not read unsigned 24 bit integer.");
}

/// <summary>
Expand Down Expand Up @@ -303,12 +307,13 @@ public static unsafe void WriteUInt24LittleEndian(Stream stream, uint data)
/// </summary>
/// <param name="stream">The stream to read the data from.</param>
/// <param name="buffer">Buffer to store the data read from the stream.</param>
/// <param name="required">If true, the chunk size is required to be read, otherwise it can be skipped.</param>
/// <returns>The chunk size in bytes.</returns>
/// <exception cref="ImageFormatException">Thrown if the input stream is not valid.</exception>
public static uint ReadChunkSize(BufferedReadStream stream, Span<byte> buffer, bool required = true)
/// <exception cref="InvalidImageContentException">Thrown if the stream ends inside the size field.</exception>
public static uint ReadChunkSize(BufferedReadStream stream, Span<byte> buffer)
{
ulong chunkSize = ReadPaddedChunkSize(stream, buffer, required);
// Widen before padding so an odd uint.MaxValue payload cannot wrap to zero.
ulong chunkSize = ReadChunkPayloadSize(stream, buffer, true);
chunkSize += chunkSize & 1;

// Structural chunk sizes must remain representable by their uint-sized consumers.
// Metadata readers retain the wider extent so their recovery can skip it safely.
Expand All @@ -321,23 +326,24 @@ public static uint ReadChunkSize(BufferedReadStream stream, Span<byte> buffer, b
}

/// <summary>
/// Reads a chunk's complete padded extent without wrapping a uint-sized payload length.
/// Reads the payload size stored in a chunk header. RIFF follows an odd-sized payload with a padding byte
/// that belongs to the chunk but not to its payload, so the returned size excludes it.
/// </summary>
/// <param name="stream">The input stream.</param>
/// <param name="buffer">The four-byte size buffer.</param>
/// <param name="required">Whether an incomplete size field is an error.</param>
/// <returns>The padded extent, or remaining bytes when an optional size field is incomplete.</returns>
private static ulong ReadPaddedChunkSize(BufferedReadStream stream, Span<byte> buffer, bool required)
/// <returns>The payload size, or remaining bytes when an optional size field is incomplete.</returns>
private static ulong ReadChunkPayloadSize(BufferedReadStream stream, Span<byte> buffer, bool required)
{
if (stream.Read(buffer) is 4)
{
uint chunkSize = BinaryPrimitives.ReadUInt32LittleEndian(buffer);
return (ulong)chunkSize + (chunkSize & 1);
return BinaryPrimitives.ReadUInt32LittleEndian(buffer);
}

if (required)
{
throw new ImageFormatException("Invalid Webp data, could not read chunk size.");
// Only a truncated stream leaves fewer than four bytes for the size field.
throw new InvalidImageContentException("Invalid Webp data, could not read chunk size.");
}

// Return the size of the remaining data in the stream.
Expand All @@ -349,8 +355,8 @@ private static ulong ReadPaddedChunkSize(BufferedReadStream stream, Span<byte> b
/// </summary>
/// <param name="stream">The stream to read the data from.</param>
/// <param name="buffer">Buffer to store the data read from the stream.</param>
/// <exception cref="ImageFormatException">
/// Thrown if the input stream is not valid.
/// <exception cref="InvalidImageContentException">
/// Thrown if the stream ends inside the chunk type.
/// </exception>
public static WebpChunkType ReadChunkType(BufferedReadStream stream, Span<byte> buffer)
{
Expand All @@ -360,8 +366,8 @@ public static WebpChunkType ReadChunkType(BufferedReadStream stream, Span<byte>
}

// While we ignore unknown chunks we still need a to be a ble to read a chunk type
// known or otherwise from the stream.
throw new ImageFormatException("Invalid Webp data, could not read chunk type.");
// known or otherwise from the stream. Only a truncated stream leaves fewer than four bytes.
throw new InvalidImageContentException("Invalid Webp data, could not read chunk type.");
}

/// <summary>
Expand All @@ -377,18 +383,18 @@ public static void ReadIccProfile(
bool ignoreMetadata,
Action<Action> executeAncillarySegmentAction)
{
ulong chunkSize = ReadPaddedChunkSize(stream, stackalloc byte[4], true);
ulong payloadSize = ReadChunkPayloadSize(stream, stackalloc byte[4], true);

// ICCP precedes image/frame data. Its framing must be readable even when
// metadata is skipped; otherwise there is no safe location to resume decoding.
if (!stream.IsReadRangeValid(stream.Position, chunkSize))
// ICCP precedes image/frame data. Its framing, including the padding byte of an odd-sized payload,
// must be readable even when metadata is skipped; otherwise there is no safe location to resume decoding.
if (!stream.IsReadRangeValid(stream.Position, payloadSize + (payloadSize & 1)))
{
WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the ICCP chunk.");
}

executeAncillarySegmentAction(() =>
{
byte[]? iccpData = ReadMetadataChunk(stream, chunkSize, ignoreMetadata || metadata.IccProfile != null);
byte[]? iccpData = ReadMetadataChunk(stream, payloadSize, ignoreMetadata || metadata.IccProfile != null);
if (iccpData is not null)
{
IccProfile profile = new(iccpData);
Expand All @@ -413,8 +419,8 @@ public static void ReadExifProfile(
ImageMetadata metadata,
bool ignoreMetadata)
{
ulong chunkSize = ReadPaddedChunkSize(stream, stackalloc byte[4], !ignoreMetadata);
byte[]? exifData = ReadMetadataChunk(stream, chunkSize, ignoreMetadata || metadata.ExifProfile != null);
ulong payloadSize = ReadChunkPayloadSize(stream, stackalloc byte[4], !ignoreMetadata);
byte[]? exifData = ReadMetadataChunk(stream, payloadSize, ignoreMetadata || metadata.ExifProfile != null);
if (exifData is not null)
{
ExifProfile exifProfile = new(exifData);
Expand Down Expand Up @@ -445,8 +451,8 @@ public static void ReadXmpProfile(
ImageMetadata metadata,
bool ignoreMetadata)
{
ulong chunkSize = ReadPaddedChunkSize(stream, stackalloc byte[4], !ignoreMetadata);
byte[]? xmpData = ReadMetadataChunk(stream, chunkSize, ignoreMetadata || metadata.XmpProfile != null);
ulong payloadSize = ReadChunkPayloadSize(stream, stackalloc byte[4], !ignoreMetadata);
byte[]? xmpData = ReadMetadataChunk(stream, payloadSize, ignoreMetadata || metadata.XmpProfile != null);
if (xmpData is not null)
{
metadata.XmpProfile = new XmpProfile(xmpData);
Expand All @@ -458,19 +464,22 @@ public static void ReadXmpProfile(
/// Callers execute metadata parsing under the decoder's ancillary integrity policy.
/// </summary>
/// <param name="stream">The input stream positioned at the chunk payload.</param>
/// <param name="paddedLength">The declared extent including its padding byte.</param>
/// <param name="payloadLength">The declared payload length, excluding its padding byte.</param>
/// <param name="ignoreMetadata">Whether to skip the payload without parsing it.</param>
/// <returns>The payload, or null when metadata is skipped.</returns>
private static byte[]? ReadMetadataChunk(BufferedReadStream stream, ulong paddedLength, bool ignoreMetadata)
private static byte[]? ReadMetadataChunk(BufferedReadStream stream, ulong payloadLength, bool ignoreMetadata)
{
// An odd-sized payload is followed by a padding byte that is part of the chunk but not of the payload.
// The next chunk starts after it, clamped to the end of a truncated stream.
ulong paddedLength = payloadLength + (payloadLength & 1);
long chunkEnd = stream.Position + (long)Math.Min(paddedLength, (ulong)stream.RemainingBytes);
if (ignoreMetadata)
{
stream.Position = chunkEnd;
return null;
}

if (!stream.TryGetReadLength(paddedLength, out int bufferLength))
if (!stream.TryGetReadLength(payloadLength, out int bufferLength))
{
// Ignoring an ancillary error must not make the next parser interpret
// this payload as another chunk header. A truncated chunk consumes EOF.
Expand All @@ -484,6 +493,8 @@ public static void ReadXmpProfile(
WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the metadata chunk.");
}

// Skip the padding byte, so it is neither returned as payload nor read as the next chunk header.
stream.Position = chunkEnd;
return data;
}

Expand Down
38 changes: 27 additions & 11 deletions src/ImageSharp/Formats/Webp/WebpDecoderCore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ internal sealed class WebpDecoderCore : ImageDecoderCore, IDisposable
/// </summary>
private readonly BackgroundColorHandling backgroundColorHandling;

/// <summary>
/// The stream position where the RIFF payload ends. Chunk parsing stops here, so bytes after the payload are ignored.
/// </summary>
private long riffEnd;

/// <summary>
/// Initializes a new instance of the <see cref="WebpDecoderCore"/> class.
/// </summary>
Expand All @@ -75,7 +80,7 @@ protected override Image<TPixel> Decode<TPixel>(BufferedReadStream stream, Cance
ImageMetadata metadata = new();
Span<byte> buffer = stackalloc byte[4];

uint fileSize = ReadImageHeader(stream, buffer);
uint fileSize = this.ReadImageHeader(stream, buffer);

using (this.webImageInfo = this.ReadVp8Info(stream, metadata))
{
Expand Down Expand Up @@ -140,7 +145,7 @@ protected override Image<TPixel> Decode<TPixel>(BufferedReadStream stream, Cance
/// <inheritdoc />
protected override ImageInfo Identify(BufferedReadStream stream, CancellationToken cancellationToken)
{
uint fileSize = ReadImageHeader(stream, stackalloc byte[4]);
uint fileSize = this.ReadImageHeader(stream, stackalloc byte[4]);
ImageMetadata metadata = new();

using (this.webImageInfo = this.ReadVp8Info(stream, metadata, true))
Expand Down Expand Up @@ -170,13 +175,15 @@ protected override ImageInfo Identify(BufferedReadStream stream, CancellationTok
}

/// <summary>
/// Reads and skips over the image header.
/// Reads and skips over the image header, and records where the RIFF payload ends.
/// </summary>
/// <param name="stream">The stream to decode from.</param>
/// <param name="buffer">Temporary buffer.</param>
/// <returns>The file size in bytes.</returns>
private static uint ReadImageHeader(BufferedReadStream stream, Span<byte> buffer)
private uint ReadImageHeader(BufferedReadStream stream, Span<byte> buffer)
{
long riffStart = stream.Position;

// Skip FourCC header, we already know its a RIFF file at this point.
stream.Skip(4);

Expand All @@ -185,6 +192,11 @@ private static uint ReadImageHeader(BufferedReadStream stream, Span<byte> buffer
// The file size in the header is the total size of the chunks that follow plus 4 bytes for the ‘WEBP’ FourCC.
uint fileSize = WebpChunkParsingUtils.ReadChunkSize(stream, buffer);

// Bytes after the RIFF payload are not part of the image, so chunks are only read up to its end,
// as libwebp does. A declared size larger than the stream describes a truncated file, so reading
// then stops at the end of the stream instead.
this.riffEnd = Math.Min(riffStart + WebpConstants.ChunkHeaderSize + fileSize, stream.Length);

// Skip 'WEBP' from the header.
stream.Skip(4);

Expand Down Expand Up @@ -221,7 +233,9 @@ private WebpImageInfo ReadVp8Info(BufferedReadStream stream, ImageMetadata metad
return info;
case WebpChunkType.Vp8X:
info = WebpChunkParsingUtils.ReadVp8XHeader(stream, buffer, features);
while (stream.Position < stream.Length)

// Fewer bytes than a chunk header before the end of the RIFF payload cannot start another chunk.
while (this.riffEnd - stream.Position >= WebpConstants.ChunkHeaderSize)
{
chunkType = WebpChunkParsingUtils.ReadChunkType(stream, buffer);
if (chunkType == WebpChunkType.Vp8)
Expand Down Expand Up @@ -250,8 +264,9 @@ private WebpImageInfo ReadVp8Info(BufferedReadStream stream, ImageMetadata metad
{
// Ignore unknown chunks.
// These must always fall after the image data so we are safe to always skip them.
uint chunkSize = WebpChunkParsingUtils.ReadChunkSize(stream, buffer, false);
stream.Skip((int)chunkSize);
// A chunk extending past the RIFF payload ends the parsing instead of wrapping to a negative skip.
uint chunkSize = WebpChunkParsingUtils.ReadChunkSize(stream, buffer);
stream.Position = Math.Min(stream.Position + chunkSize, this.riffEnd);
}
}

Expand Down Expand Up @@ -334,8 +349,8 @@ private void ParseOptionalChunks(BufferedReadStream stream, ImageMetadata metada
return;
}

long streamLength = stream.Length;
while (stream.Position < streamLength)
// Fewer bytes than a chunk header before the end of the RIFF payload cannot start another chunk.
while (this.riffEnd - stream.Position >= WebpConstants.ChunkHeaderSize)
{
// Read chunk header.
WebpChunkType chunkType = WebpChunkParsingUtils.ReadChunkType(stream, buffer);
Expand All @@ -350,8 +365,9 @@ private void ParseOptionalChunks(BufferedReadStream stream, ImageMetadata metada
else
{
// Skip unknown chunks.
uint chunkLength = WebpChunkParsingUtils.ReadChunkSize(stream, buffer, false);
stream.Skip((int)chunkLength);
// A chunk extending past the RIFF payload ends the parsing instead of wrapping to a negative skip.
uint chunkLength = WebpChunkParsingUtils.ReadChunkSize(stream, buffer);
stream.Position = Math.Min(stream.Position + chunkLength, this.riffEnd);
}
}
}
Expand Down
Loading