From 0f9f1bf160d342b2ed01587d37aa426c8cf0d770 Mon Sep 17 00:00:00 2001 From: Sven Claesson Date: Sun, 10 May 2026 12:59:37 +0200 Subject: [PATCH 1/3] Fix integer overflow and bounds-checking vulnerabilities in EXR decoder Use ulong arithmetic in CalculateBytesPerRow and block size calculations to prevent integer overflow. Add validation for DataWindow dimensions, block size limits, and row offsets outside stream bounds. Co-Authored-By: Claude Sonnet 4.6 --- src/ImageSharp/Formats/Exr/ExrDecoderCore.cs | 53 ++++- src/ImageSharp/Formats/Exr/ExrEncoderCore.cs | 4 +- src/ImageSharp/Formats/Exr/ExrUtils.cs | 8 +- .../Formats/Exr/ExrDecoderSecurityTests.cs | 220 ++++++++++++++++++ 4 files changed, 269 insertions(+), 16 deletions(-) create mode 100644 tests/ImageSharp.Tests/Formats/Exr/ExrDecoderSecurityTests.cs diff --git a/src/ImageSharp/Formats/Exr/ExrDecoderCore.cs b/src/ImageSharp/Formats/Exr/ExrDecoderCore.cs index 2e60f3a5b9..4bdf58e49b 100644 --- a/src/ImageSharp/Formats/Exr/ExrDecoderCore.cs +++ b/src/ImageSharp/Formats/Exr/ExrDecoderCore.cs @@ -139,9 +139,14 @@ private void DecodeFloatingPointPixelData(BufferedReadStream stream, Buf where TPixel : unmanaged, IPixel { bool hasAlpha = this.HasAlpha(); - uint bytesPerRow = ExrUtils.CalculateBytesPerRow(this.Channels, (uint)this.Width); + ulong bytesPerRow = ExrUtils.CalculateBytesPerRow(this.Channels, (uint)this.Width); uint rowsPerBlock = ExrUtils.RowsPerBlock(this.Compression); - uint bytesPerBlock = bytesPerRow * rowsPerBlock; + ulong bytesPerBlock = bytesPerRow * rowsPerBlock; + if (bytesPerBlock > int.MaxValue) + { + ExrThrowHelper.ThrowInvalidImageContentException("EXR block size exceeds the maximum allowed size."); + } + int width = this.Width; int height = this.Height; int channelCount = this.Channels.Count; @@ -158,8 +163,8 @@ private void DecodeFloatingPointPixelData(BufferedReadStream stream, Buf this.Compression, this.memoryAllocator, width, - bytesPerBlock, - bytesPerRow, + (uint)bytesPerBlock, + (uint)bytesPerRow, rowsPerBlock, channelCount, this.PixelType); @@ -170,6 +175,11 @@ private void DecodeFloatingPointPixelData(BufferedReadStream stream, Buf ulong rowOffset = this.ReadUnsignedLong(stream); long nextRowOffsetPosition = stream.Position; + if (rowOffset >= (ulong)stream.Length) + { + ExrThrowHelper.ThrowInvalidImageContentException("EXR row offset is outside the bounds of the stream."); + } + stream.Position = (long)rowOffset; uint rowStartIndex = this.ReadUnsignedInteger(stream); @@ -212,9 +222,14 @@ private void DecodeUnsignedIntPixelData(BufferedReadStream stream, Buffe where TPixel : unmanaged, IPixel { bool hasAlpha = this.HasAlpha(); - uint bytesPerRow = ExrUtils.CalculateBytesPerRow(this.Channels, (uint)this.Width); + ulong bytesPerRow = ExrUtils.CalculateBytesPerRow(this.Channels, (uint)this.Width); uint rowsPerBlock = ExrUtils.RowsPerBlock(this.Compression); - uint bytesPerBlock = bytesPerRow * rowsPerBlock; + ulong bytesPerBlock = bytesPerRow * rowsPerBlock; + if (bytesPerBlock > int.MaxValue) + { + ExrThrowHelper.ThrowInvalidImageContentException("EXR block size exceeds the maximum allowed size."); + } + int width = this.Width; int height = this.Height; int channelCount = this.Channels.Count; @@ -231,8 +246,8 @@ private void DecodeUnsignedIntPixelData(BufferedReadStream stream, Buffe this.Compression, this.memoryAllocator, width, - bytesPerBlock, - bytesPerRow, + (uint)bytesPerBlock, + (uint)bytesPerRow, rowsPerBlock, channelCount, this.PixelType); @@ -243,6 +258,11 @@ private void DecodeUnsignedIntPixelData(BufferedReadStream stream, Buffe ulong rowOffset = this.ReadUnsignedLong(stream); long nextRowOffsetPosition = stream.Position; + if (rowOffset >= (ulong)stream.Length) + { + ExrThrowHelper.ThrowInvalidImageContentException("EXR row offset is outside the bounds of the stream."); + } + stream.Position = (long)rowOffset; uint rowStartIndex = this.ReadUnsignedInteger(stream); @@ -597,8 +617,21 @@ private ExrHeaderAttributes ReadExrHeader(BufferedReadStream stream) this.HeaderAttributes = this.ParseHeaderAttributes(stream); - this.Width = this.HeaderAttributes.DataWindow.XMax - this.HeaderAttributes.DataWindow.XMin + 1; - this.Height = this.HeaderAttributes.DataWindow.YMax - this.HeaderAttributes.DataWindow.YMin + 1; + ExrBox2i dataWindow = this.HeaderAttributes.DataWindow; + if (dataWindow.XMax < dataWindow.XMin || dataWindow.YMax < dataWindow.YMin) + { + ExrThrowHelper.ThrowInvalidImageContentException("EXR DataWindow max values must be greater than or equal to min values."); + } + + long width = (long)dataWindow.XMax - dataWindow.XMin + 1; + long height = (long)dataWindow.YMax - dataWindow.YMin + 1; + if (width > int.MaxValue || height > int.MaxValue) + { + ExrThrowHelper.ThrowInvalidImageContentException("EXR DataWindow dimensions exceed the maximum allowed size."); + } + + this.Width = (int)width; + this.Height = (int)height; this.Channels = this.HeaderAttributes.Channels; this.Compression = this.HeaderAttributes.Compression; this.PixelType = this.ValidateChannels(); diff --git a/src/ImageSharp/Formats/Exr/ExrEncoderCore.cs b/src/ImageSharp/Formats/Exr/ExrEncoderCore.cs index e2750d8972..bb696cca1d 100644 --- a/src/ImageSharp/Formats/Exr/ExrEncoderCore.cs +++ b/src/ImageSharp/Formats/Exr/ExrEncoderCore.cs @@ -170,7 +170,7 @@ private ulong[] EncodeFloatingPointPixelData( CancellationToken cancellationToken) where TPixel : unmanaged, IPixel { - uint bytesPerRow = ExrUtils.CalculateBytesPerRow(channels, (uint)width); + uint bytesPerRow = (uint)ExrUtils.CalculateBytesPerRow(channels, (uint)width); uint rowsPerBlock = ExrUtils.RowsPerBlock(compression); uint bytesPerBlock = bytesPerRow * rowsPerBlock; @@ -262,7 +262,7 @@ private ulong[] EncodeUnsignedIntPixelData( CancellationToken cancellationToken) where TPixel : unmanaged, IPixel { - uint bytesPerRow = ExrUtils.CalculateBytesPerRow(channels, (uint)width); + uint bytesPerRow = (uint)ExrUtils.CalculateBytesPerRow(channels, (uint)width); uint rowsPerBlock = ExrUtils.RowsPerBlock(compression); uint bytesPerBlock = bytesPerRow * rowsPerBlock; diff --git a/src/ImageSharp/Formats/Exr/ExrUtils.cs b/src/ImageSharp/Formats/Exr/ExrUtils.cs index 386210b81d..532950daa9 100644 --- a/src/ImageSharp/Formats/Exr/ExrUtils.cs +++ b/src/ImageSharp/Formats/Exr/ExrUtils.cs @@ -13,9 +13,9 @@ internal static class ExrUtils /// The image channels array. /// The width in pixels of a row. /// The number of bytes per row. - public static uint CalculateBytesPerRow(IList channels, uint width) + public static ulong CalculateBytesPerRow(IList channels, uint width) { - uint bytesPerRow = 0; + ulong bytesPerRow = 0; foreach (ExrChannelInfo channelInfo in channels) { if (channelInfo.ChannelName.Equals("A", StringComparison.Ordinal) @@ -26,11 +26,11 @@ public static uint CalculateBytesPerRow(IList channels, uint wid { if (channelInfo.PixelType == ExrPixelType.Half) { - bytesPerRow += 2 * width; + bytesPerRow += 2UL * width; } else { - bytesPerRow += 4 * width; + bytesPerRow += 4UL * width; } } } diff --git a/tests/ImageSharp.Tests/Formats/Exr/ExrDecoderSecurityTests.cs b/tests/ImageSharp.Tests/Formats/Exr/ExrDecoderSecurityTests.cs new file mode 100644 index 0000000000..e2ea8121e4 --- /dev/null +++ b/tests/ImageSharp.Tests/Formats/Exr/ExrDecoderSecurityTests.cs @@ -0,0 +1,220 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using System.Buffers.Binary; +using SixLabors.ImageSharp.Formats; +using SixLabors.ImageSharp.Formats.Exr; +using SixLabors.ImageSharp.PixelFormats; + +namespace SixLabors.ImageSharp.Tests.Formats.Exr; + +/// +/// Security regression tests for the EXR decoder (Findings EXR-1, EXR-2, EXR-3). +/// The EXR decoder was merged to main but not yet included in a tagged NuGet release. +/// Each test demonstrates a crafted-input crash present in the unfixed code. +/// +public class ExrDecoderSecurityTests +{ + /// + /// EXR-1 — EXR DataWindow Integer Overflow Produces Negative Image Dimensions (DoS) + /// + /// Width and Height are computed from attacker-controlled DataWindow attributes + /// using unchecked int subtraction: + /// this.Width = XMax - XMin + 1 // overflows to -2147483647 + /// + /// The negative Width is then passed to the Image<TPixel> constructor, which calls + /// Guard.MustBeGreaterThan(width, 0) → ArgumentOutOfRangeException. + /// + /// After a fix this should throw InvalidImageContentException instead. + /// + /// Affected file: + /// src/ImageSharp/Formats/Exr/ExrDecoderCore.cs lines 600–601 + /// + [Fact] + public void Decode_DataWindowOverflow_NegativeWidth_Throws() + { + // XMin = -1073741825, XMax = 1073741823 + // Width = 1073741823 - (-1073741825) + 1 = 2^31 + 1 → wraps to -2147483647 + byte[] data = BuildMinimalExr(xMin: -1073741825, yMin: 0, xMax: 1073741823, yMax: 0); + + using var stream = new MemoryStream(data); + Assert.Throws( + () => ExrDecoder.Instance.Decode(DecoderOptions.Default, stream)); + } + + /// + /// EXR-2 — EXR Row Offset Table Unvalidated Seek (DoS / data integrity) + /// + /// Row offsets are read from the file and used unconditionally to seek the stream: + /// ulong rowOffset = this.ReadUnsignedLong(stream); + /// stream.Position = (long)rowOffset; // no bounds check + /// + /// A crafted offset of 0xFFFFFFFFFFFFFFFF casts to −1 as long, causing + /// an ArgumentOutOfRangeException when setting stream.Position. + /// + /// After a fix this should throw InvalidImageContentException instead. + /// + /// Affected file: + /// src/ImageSharp/Formats/Exr/ExrDecoderCore.cs lines 170–175 (scanline) + /// lines 243–248 (tile) + /// + [Fact] + public void Decode_CraftedRowOffsets_OutOfBounds_Throws() + { + // Valid 2×2 image (XMin=0,YMin=0,XMax=1,YMax=1 → Width=2,Height=2). + // Row offset table immediately follows the header null byte: + // 2 rows × 8 bytes each, all set to 0xFFFFFFFFFFFFFFFF. + byte[] invalidOffsets = new byte[16]; + BinaryPrimitives.WriteUInt64LittleEndian(invalidOffsets, 0xFFFFFFFFFFFFFFFF); + BinaryPrimitives.WriteUInt64LittleEndian(invalidOffsets.AsSpan(8), 0xFFFFFFFFFFFFFFFF); + + byte[] data = BuildMinimalExr( + xMin: 0, yMin: 0, xMax: 1, yMax: 1, + rowOffsetTableAppend: invalidOffsets); + + using var stream = new MemoryStream(data); + Assert.Throws( + () => ExrDecoder.Instance.Decode(DecoderOptions.Default, stream)); + } + + /// + /// EXR-3 — EXR bytesPerBlock uint Overflow Chain (DoS) + /// + /// CalculateBytesPerRow is computed in ulong (fixed), and if the result exceeds + /// int.MaxValue the decoder throws InvalidImageContentException. With 4 RGBA HALF + /// channels and Width = 2^29: + /// bytesPerRow = 4 × 2 × 2^29 = 2^32 (> int.MaxValue) + /// → InvalidImageContentException before any allocation + /// + /// Affected file: + /// src/ImageSharp/Formats/Exr/ExrDecoderCore.cs lines 142–150, 215–223 + /// src/ImageSharp/Formats/Exr/ExrUtils.cs CalculateBytesPerRow + /// + [Fact] + public void Decode_BytesPerBlockUintOverflow_Throws() + { + // 4 RGBA HALF channels, Width = 2^29: + // bytesPerRow = 4 × 2 × 536870912 = 4294967296 > int.MaxValue + // → InvalidImageContentException from the block-size guard + byte[] data = BuildMinimalRgbaExr(xMin: 0, yMin: 0, xMax: 536870911, yMax: 0); + + using var stream = new MemoryStream(data); + Assert.Throws( + () => ExrDecoder.Instance.Decode(DecoderOptions.Default, stream)); + } + + // ------------------------------------------------------------------------- + // Helpers: construct minimal valid-enough EXR scanline files. + // + // Required attributes per ParseHeaderAttributes validation: + // channels, compression, dataWindow, displayWindow, + // lineOrder, pixelAspectRatio, screenWindowCenter, screenWindowWidth + // ------------------------------------------------------------------------- + + private static byte[] BuildMinimalExr( + int xMin, int yMin, int xMax, int yMax, + byte[]? rowOffsetTableAppend = null) + { + // channels: single "R" HALF channel with xSampling=1, ySampling=1 + // Layout per ReadChannelInfo: name\0 (2) + pixelType (4) + pLinear+reserved (4) + // + xSampling (4) + ySampling (4) = 18 bytes/channel + // + list-null (1) = 19 total + byte[] channelData = + [ + 0x52, 0x00, // "R\0" + 0x01, 0x00, 0x00, 0x00, // pixelType = Half (1) + 0x00, 0x00, 0x00, 0x00, // pLinear + 3 reserved bytes + 0x01, 0x00, 0x00, 0x00, // xSampling = 1 + 0x01, 0x00, 0x00, 0x00, // ySampling = 1 + 0x00, // channel-list null terminator + ]; + + return BuildExrWithChannels(xMin, yMin, xMax, yMax, channelData, rowOffsetTableAppend); + } + + private static byte[] BuildMinimalRgbaExr(int xMin, int yMin, int xMax, int yMax) + { + // 4 HALF channels in alphabetical order (A, B, G, R) per EXR spec. + // 18 bytes per channel × 4 channels + 1 list-null = 73 bytes total. + byte[] channelData = + [ + 0x41, 0x00, // "A\0" + 0x01, 0x00, 0x00, 0x00, // pixelType = Half (1) + 0x00, 0x00, 0x00, 0x00, // pLinear + 3 reserved bytes + 0x01, 0x00, 0x00, 0x00, // xSampling = 1 + 0x01, 0x00, 0x00, 0x00, // ySampling = 1 + 0x42, 0x00, // "B\0" + 0x01, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x00, + 0x47, 0x00, // "G\0" + 0x01, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x00, + 0x52, 0x00, // "R\0" + 0x01, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x00, + 0x00, // channel-list null terminator + ]; + + return BuildExrWithChannels(xMin, yMin, xMax, yMax, channelData); + } + + private static byte[] BuildExrWithChannels( + int xMin, int yMin, int xMax, int yMax, + byte[] channelData, + byte[]? rowOffsetTableAppend = null) + { + using var ms = new MemoryStream(); + using var bw = new BinaryWriter(ms, System.Text.Encoding.ASCII, leaveOpen: true); + + // Magic (0x01312F76 LE) + version 2, scanline (flags = 0x00) + bw.Write(new byte[] { 0x76, 0x2F, 0x31, 0x01, 0x02, 0x00, 0x00, 0x00 }); + + void WriteAttr(string name, string type, byte[] payload) + { + foreach (char c in name) bw.Write((byte)c); + bw.Write((byte)0); + foreach (char c in type) bw.Write((byte)c); + bw.Write((byte)0); + bw.Write(payload.Length); + bw.Write(payload); + } + + WriteAttr("channels", "chlist", channelData); + + WriteAttr("compression", "compression", [0x00]); // None + + byte[] dw = new byte[16]; + BinaryPrimitives.WriteInt32LittleEndian(dw, xMin); + BinaryPrimitives.WriteInt32LittleEndian(dw.AsSpan(4), yMin); + BinaryPrimitives.WriteInt32LittleEndian(dw.AsSpan(8), xMax); + BinaryPrimitives.WriteInt32LittleEndian(dw.AsSpan(12), yMax); + WriteAttr("dataWindow", "box2i", dw); + + WriteAttr("displayWindow", "box2i", new byte[16]); // all zeros (0,0,0,0) + + WriteAttr("lineOrder", "lineOrder", [0x00]); // IncreasingY + + byte[] aspect = new byte[4]; + BinaryPrimitives.WriteSingleLittleEndian(aspect, 1.0f); + WriteAttr("pixelAspectRatio", "float", aspect); + + WriteAttr("screenWindowCenter", "v2f", new byte[8]); // (0f, 0f) + + byte[] sww = new byte[4]; + BinaryPrimitives.WriteSingleLittleEndian(sww, 1.0f); + WriteAttr("screenWindowWidth", "float", sww); + + bw.Write((byte)0x00); // end-of-header sentinel + + if (rowOffsetTableAppend is not null) + bw.Write(rowOffsetTableAppend); + + return ms.ToArray(); + } +} From c1cf0e9788a59925f7405e91a783c98c5b34de4a Mon Sep 17 00:00:00 2001 From: Sven Claesson Date: Sun, 10 May 2026 13:28:10 +0200 Subject: [PATCH 2/3] Harden EXR row offset validation --- src/ImageSharp/Formats/Exr/ExrDecoderCore.cs | 33 +++++++++++++------ .../Formats/Exr/ExrDecoderSecurityTests.cs | 33 ++++++++++++++++++- 2 files changed, 55 insertions(+), 11 deletions(-) diff --git a/src/ImageSharp/Formats/Exr/ExrDecoderCore.cs b/src/ImageSharp/Formats/Exr/ExrDecoderCore.cs index 4bdf58e49b..34ccbc1280 100644 --- a/src/ImageSharp/Formats/Exr/ExrDecoderCore.cs +++ b/src/ImageSharp/Formats/Exr/ExrDecoderCore.cs @@ -91,6 +91,11 @@ public ExrDecoderCore(ExrDecoderOptions options) /// private ExrHeaderAttributes HeaderAttributes { get; set; } + /// + /// Gets or sets the earliest valid stream position for a scanline chunk. + /// + private long MinimumChunkOffset { get; set; } + /// protected override Image Decode(BufferedReadStream stream, CancellationToken cancellationToken) { @@ -175,11 +180,7 @@ private void DecodeFloatingPointPixelData(BufferedReadStream stream, Buf ulong rowOffset = this.ReadUnsignedLong(stream); long nextRowOffsetPosition = stream.Position; - if (rowOffset >= (ulong)stream.Length) - { - ExrThrowHelper.ThrowInvalidImageContentException("EXR row offset is outside the bounds of the stream."); - } - + this.ValidateChunkOffset(rowOffset, stream); stream.Position = (long)rowOffset; uint rowStartIndex = this.ReadUnsignedInteger(stream); @@ -258,11 +259,7 @@ private void DecodeUnsignedIntPixelData(BufferedReadStream stream, Buffe ulong rowOffset = this.ReadUnsignedLong(stream); long nextRowOffsetPosition = stream.Position; - if (rowOffset >= (ulong)stream.Length) - { - ExrThrowHelper.ThrowInvalidImageContentException("EXR row offset is outside the bounds of the stream."); - } - + this.ValidateChunkOffset(rowOffset, stream); stream.Position = (long)rowOffset; uint rowStartIndex = this.ReadUnsignedInteger(stream); @@ -634,6 +631,9 @@ private ExrHeaderAttributes ReadExrHeader(BufferedReadStream stream) this.Height = (int)height; this.Channels = this.HeaderAttributes.Channels; this.Compression = this.HeaderAttributes.Compression; + uint rowsPerBlock = ExrUtils.RowsPerBlock(this.Compression); + long chunkCount = (this.Height + (long)rowsPerBlock - 1) / rowsPerBlock; + this.MinimumChunkOffset = stream.Position + (chunkCount * sizeof(ulong)); this.PixelType = this.ValidateChannels(); this.ImageDataType = this.DetermineImageDataType(); @@ -899,6 +899,19 @@ private static string ReadString(BufferedReadStream stream) _ => false, }; + /// + /// Validates a scanline chunk offset read from the EXR offset table. + /// + /// The chunk offset to validate. + /// The stream containing the image data. + private void ValidateChunkOffset(ulong chunkOffset, BufferedReadStream stream) + { + if (chunkOffset < (ulong)this.MinimumChunkOffset || chunkOffset >= (ulong)stream.Length) + { + ExrThrowHelper.ThrowInvalidImageContentException("EXR chunk offset is outside the bounds of the stream."); + } + } + /// /// Determines whether this image has alpha channel. /// diff --git a/tests/ImageSharp.Tests/Formats/Exr/ExrDecoderSecurityTests.cs b/tests/ImageSharp.Tests/Formats/Exr/ExrDecoderSecurityTests.cs index e2ea8121e4..70f3d577e9 100644 --- a/tests/ImageSharp.Tests/Formats/Exr/ExrDecoderSecurityTests.cs +++ b/tests/ImageSharp.Tests/Formats/Exr/ExrDecoderSecurityTests.cs @@ -43,7 +43,7 @@ public void Decode_DataWindowOverflow_NegativeWidth_Throws() } /// - /// EXR-2 — EXR Row Offset Table Unvalidated Seek (DoS / data integrity) + /// EXR-2 — EXR Row Offset Table Unvalidated Seek (DoS) /// /// Row offsets are read from the file and used unconditionally to seek the stream: /// ulong rowOffset = this.ReadUnsignedLong(stream); @@ -77,6 +77,37 @@ public void Decode_CraftedRowOffsets_OutOfBounds_Throws() () => ExrDecoder.Instance.Decode(DecoderOptions.Default, stream)); } + [Fact] + public void Decode_CraftedRowOffsets_IntoHeader_Throws() + { + // Offset 0 points back into the EXR file header and must be rejected + // before the decoder seeks to attacker-controlled non-pixel data. + byte[] headerOffsets = new byte[16]; + + byte[] data = BuildMinimalExr( + xMin: 0, yMin: 0, xMax: 1, yMax: 1, + rowOffsetTableAppend: headerOffsets); + + using var stream = new MemoryStream(data); + Assert.Throws( + () => ExrDecoder.Instance.Decode(DecoderOptions.Default, stream)); + } + + [Fact] + public void Decode_CraftedRowOffsets_IntoOffsetTable_Throws() + { + byte[] data = BuildMinimalExr( + xMin: 0, yMin: 0, xMax: 1, yMax: 1, + rowOffsetTableAppend: new byte[16]); + + // Point the first row offset at the second row offset entry. + BinaryPrimitives.WriteUInt64LittleEndian(data.AsSpan(data.Length - 16), (ulong)(data.Length - 8)); + + using var stream = new MemoryStream(data); + Assert.Throws( + () => ExrDecoder.Instance.Decode(DecoderOptions.Default, stream)); + } + /// /// EXR-3 — EXR bytesPerBlock uint Overflow Chain (DoS) /// From e144a59dbb3e8277894d0b8ef49ce8d1a0405d13 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 12 May 2026 17:21:18 +1000 Subject: [PATCH 3/3] EXR: validate sizes, prevent overflows, dispose image --- src/ImageSharp/Formats/Exr/ExrDecoderCore.cs | 59 +++++++++++++------ src/ImageSharp/Formats/Exr/ExrEncoderCore.cs | 20 +++++-- .../Formats/Exr/ExrDecoderSecurityTests.cs | 47 +++++++++++---- 3 files changed, 89 insertions(+), 37 deletions(-) diff --git a/src/ImageSharp/Formats/Exr/ExrDecoderCore.cs b/src/ImageSharp/Formats/Exr/ExrDecoderCore.cs index 34ccbc1280..c24352d576 100644 --- a/src/ImageSharp/Formats/Exr/ExrDecoderCore.cs +++ b/src/ImageSharp/Formats/Exr/ExrDecoderCore.cs @@ -105,24 +105,33 @@ protected override Image Decode(BufferedReadStream stream, Cance ExrThrowHelper.ThrowNotSupported($"Compression {this.Compression} is not yet supported"); } - Image image = new(this.configuration, this.Width, this.Height, this.metadata); - Buffer2D pixels = image.GetRootFramePixelBuffer(); - - switch (this.PixelType) - { - case ExrPixelType.Half: - case ExrPixelType.Float: - this.DecodeFloatingPointPixelData(stream, pixels, cancellationToken); - break; - case ExrPixelType.UnsignedInt: - this.DecodeUnsignedIntPixelData(stream, pixels, cancellationToken); - break; - default: - ExrThrowHelper.ThrowNotSupported("Pixel type is not supported"); - break; - } + Image image = null; + try + { + image = new Image(this.configuration, this.Width, this.Height, this.metadata); + Buffer2D pixels = image.GetRootFramePixelBuffer(); + + switch (this.PixelType) + { + case ExrPixelType.Half: + case ExrPixelType.Float: + this.DecodeFloatingPointPixelData(stream, pixels, cancellationToken); + break; + case ExrPixelType.UnsignedInt: + this.DecodeUnsignedIntPixelData(stream, pixels, cancellationToken); + break; + default: + ExrThrowHelper.ThrowNotSupported("Pixel type is not supported"); + break; + } - return image; + return image; + } + catch + { + image?.Dispose(); + throw; + } } /// @@ -622,7 +631,10 @@ private ExrHeaderAttributes ReadExrHeader(BufferedReadStream stream) long width = (long)dataWindow.XMax - dataWindow.XMin + 1; long height = (long)dataWindow.YMax - dataWindow.YMin + 1; - if (width > int.MaxValue || height > int.MaxValue) + + // Decoding stages each row as four color planes, so the width must be bounded + // before later width * 4 buffer sizing can overflow. + if (width > int.MaxValue / 4 || height > int.MaxValue) { ExrThrowHelper.ThrowInvalidImageContentException("EXR DataWindow dimensions exceed the maximum allowed size."); } @@ -633,7 +645,16 @@ private ExrHeaderAttributes ReadExrHeader(BufferedReadStream stream) this.Compression = this.HeaderAttributes.Compression; uint rowsPerBlock = ExrUtils.RowsPerBlock(this.Compression); long chunkCount = (this.Height + (long)rowsPerBlock - 1) / rowsPerBlock; - this.MinimumChunkOffset = stream.Position + (chunkCount * sizeof(ulong)); + long offsetTableByteCount = chunkCount * sizeof(ulong); + + // The scanline offset table sits between the header and pixel chunks; proving it + // fits in the stream keeps all later chunk offsets on the pixel-data side. + if (stream.Position > stream.Length || offsetTableByteCount > stream.Length - stream.Position) + { + ExrThrowHelper.ThrowInvalidImageContentException("EXR chunk offset table is outside the bounds of the stream."); + } + + this.MinimumChunkOffset = stream.Position + offsetTableByteCount; this.PixelType = this.ValidateChannels(); this.ImageDataType = this.DetermineImageDataType(); diff --git a/src/ImageSharp/Formats/Exr/ExrEncoderCore.cs b/src/ImageSharp/Formats/Exr/ExrEncoderCore.cs index bb696cca1d..81b7c4da11 100644 --- a/src/ImageSharp/Formats/Exr/ExrEncoderCore.cs +++ b/src/ImageSharp/Formats/Exr/ExrEncoderCore.cs @@ -170,9 +170,13 @@ private ulong[] EncodeFloatingPointPixelData( CancellationToken cancellationToken) where TPixel : unmanaged, IPixel { - uint bytesPerRow = (uint)ExrUtils.CalculateBytesPerRow(channels, (uint)width); + ulong bytesPerRow = ExrUtils.CalculateBytesPerRow(channels, (uint)width); uint rowsPerBlock = ExrUtils.RowsPerBlock(compression); - uint bytesPerBlock = bytesPerRow * rowsPerBlock; + ulong bytesPerBlock = bytesPerRow * rowsPerBlock; + if (bytesPerRow > uint.MaxValue || bytesPerBlock > int.MaxValue) + { + throw new ImageFormatException("Image is too large to encode in EXR format."); + } using IMemoryOwner rgbBuffer = this.memoryAllocator.Allocate(width * 4, AllocationOptions.Clean); using IMemoryOwner rowBlockBuffer = this.memoryAllocator.Allocate((int)bytesPerBlock, AllocationOptions.Clean); @@ -181,7 +185,7 @@ private ulong[] EncodeFloatingPointPixelData( Span blueBuffer = rgbBuffer.GetSpan().Slice(width * 2, width); Span alphaBuffer = rgbBuffer.GetSpan().Slice(width * 3, width); - using ExrBaseCompressor compressor = ExrCompressorFactory.Create(compression, this.memoryAllocator, stream, bytesPerBlock, bytesPerRow, rowsPerBlock, width); + using ExrBaseCompressor compressor = ExrCompressorFactory.Create(compression, this.memoryAllocator, stream, (uint)bytesPerBlock, (uint)bytesPerRow, rowsPerBlock, width); ulong[] rowOffsets = new ulong[height]; for (uint y = 0; y < height; y += rowsPerBlock) @@ -262,9 +266,13 @@ private ulong[] EncodeUnsignedIntPixelData( CancellationToken cancellationToken) where TPixel : unmanaged, IPixel { - uint bytesPerRow = (uint)ExrUtils.CalculateBytesPerRow(channels, (uint)width); + ulong bytesPerRow = ExrUtils.CalculateBytesPerRow(channels, (uint)width); uint rowsPerBlock = ExrUtils.RowsPerBlock(compression); - uint bytesPerBlock = bytesPerRow * rowsPerBlock; + ulong bytesPerBlock = bytesPerRow * rowsPerBlock; + if (bytesPerRow > uint.MaxValue || bytesPerBlock > int.MaxValue) + { + throw new ImageFormatException("Image is too large to encode in EXR format."); + } using IMemoryOwner rgbBuffer = this.memoryAllocator.Allocate(width * 4, AllocationOptions.Clean); using IMemoryOwner rowBlockBuffer = this.memoryAllocator.Allocate((int)bytesPerBlock, AllocationOptions.Clean); @@ -273,7 +281,7 @@ private ulong[] EncodeUnsignedIntPixelData( Span blueBuffer = rgbBuffer.GetSpan().Slice(width * 2, width); Span alphaBuffer = rgbBuffer.GetSpan().Slice(width * 3, width); - using ExrBaseCompressor compressor = ExrCompressorFactory.Create(compression, this.memoryAllocator, stream, bytesPerBlock, bytesPerRow, rowsPerBlock, width); + using ExrBaseCompressor compressor = ExrCompressorFactory.Create(compression, this.memoryAllocator, stream, (uint)bytesPerBlock, (uint)bytesPerRow, rowsPerBlock, width); Rgba128 rgb = default; ulong[] rowOffsets = new ulong[height]; diff --git a/tests/ImageSharp.Tests/Formats/Exr/ExrDecoderSecurityTests.cs b/tests/ImageSharp.Tests/Formats/Exr/ExrDecoderSecurityTests.cs index 70f3d577e9..2d59c02355 100644 --- a/tests/ImageSharp.Tests/Formats/Exr/ExrDecoderSecurityTests.cs +++ b/tests/ImageSharp.Tests/Formats/Exr/ExrDecoderSecurityTests.cs @@ -13,6 +13,8 @@ namespace SixLabors.ImageSharp.Tests.Formats.Exr; /// The EXR decoder was merged to main but not yet included in a tagged NuGet release. /// Each test demonstrates a crafted-input crash present in the unfixed code. /// +[Trait("Format", "Exr")] +[ValidateDisposedMemoryAllocations] public class ExrDecoderSecurityTests { /// @@ -109,24 +111,21 @@ public void Decode_CraftedRowOffsets_IntoOffsetTable_Throws() } /// - /// EXR-3 — EXR bytesPerBlock uint Overflow Chain (DoS) + /// EXR-3 — Oversized EXR RGBA row sizing is rejected as invalid image content. /// - /// CalculateBytesPerRow is computed in ulong (fixed), and if the result exceeds - /// int.MaxValue the decoder throws InvalidImageContentException. With 4 RGBA HALF - /// channels and Width = 2^29: - /// bytesPerRow = 4 × 2 × 2^29 = 2^32 (> int.MaxValue) - /// → InvalidImageContentException before any allocation + /// With 4 RGBA HALF channels and Width = 2^29, the decoded row staging and + /// bytes-per-row arithmetic both exceed the supported buffer sizing limits. + /// The decoder must reject this as InvalidImageContentException before any allocation. /// /// Affected file: /// src/ImageSharp/Formats/Exr/ExrDecoderCore.cs lines 142–150, 215–223 /// src/ImageSharp/Formats/Exr/ExrUtils.cs CalculateBytesPerRow /// [Fact] - public void Decode_BytesPerBlockUintOverflow_Throws() + public void Decode_RgbaRowSizingExceedsBufferLimits_Throws() { - // 4 RGBA HALF channels, Width = 2^29: - // bytesPerRow = 4 × 2 × 536870912 = 4294967296 > int.MaxValue - // → InvalidImageContentException from the block-size guard + // 4 RGBA HALF channels at this width cannot be represented by the decoder's + // int-sized row staging or block buffers. byte[] data = BuildMinimalRgbaExr(xMin: 0, yMin: 0, xMax: 536870911, yMax: 0); using var stream = new MemoryStream(data); @@ -134,6 +133,30 @@ public void Decode_BytesPerBlockUintOverflow_Throws() () => ExrDecoder.Instance.Decode(DecoderOptions.Default, stream)); } + [Fact] + public void Decode_DataWindowWidthExceedsRowBufferLimit_Throws() + { + // A single HALF channel keeps bytesPerBlock below int.MaxValue, but the decoder + // still stages four color planes and must reject widths that overflow width × 4. + byte[] data = BuildMinimalExr(xMin: 0, yMin: 0, xMax: int.MaxValue / 4, yMax: 0); + + using var stream = new MemoryStream(data); + Assert.Throws( + () => ExrDecoder.Instance.Decode(DecoderOptions.Default, stream)); + } + + [Fact] + public void Identify_RowOffsetTableExceedsStream_Throws() + { + // Identify parses the header only, so this verifies the offset table bound is + // validated before scanline decoding reads from the table. + byte[] data = BuildMinimalExr(xMin: 0, yMin: 0, xMax: 1, yMax: 1); + + using var stream = new MemoryStream(data); + Assert.Throws( + () => ExrDecoder.Instance.Identify(DecoderOptions.Default, stream)); + } + // ------------------------------------------------------------------------- // Helpers: construct minimal valid-enough EXR scanline files. // @@ -144,7 +167,7 @@ public void Decode_BytesPerBlockUintOverflow_Throws() private static byte[] BuildMinimalExr( int xMin, int yMin, int xMax, int yMax, - byte[]? rowOffsetTableAppend = null) + byte[] rowOffsetTableAppend = null) { // channels: single "R" HALF channel with xSampling=1, ySampling=1 // Layout per ReadChannelInfo: name\0 (2) + pixelType (4) + pLinear+reserved (4) @@ -198,7 +221,7 @@ private static byte[] BuildMinimalRgbaExr(int xMin, int yMin, int xMax, int yMax private static byte[] BuildExrWithChannels( int xMin, int yMin, int xMax, int yMax, byte[] channelData, - byte[]? rowOffsetTableAppend = null) + byte[] rowOffsetTableAppend = null) { using var ms = new MemoryStream(); using var bw = new BinaryWriter(ms, System.Text.Encoding.ASCII, leaveOpen: true);