From 0f17a8be9c57edab885e3d7e5dc7564837be6533 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 30 Aug 2023 19:42:38 +1000 Subject: [PATCH] Handle EOF in Jpeg bit reader when data is bad to prevent DOS attack. --- .../Advanced/ParallelRowIterator.cs | 10 +++--- .../Components/Decoder/HuffmanScanBuffer.cs | 7 +++- .../Formats/Jpg/JpegDecoderTests.cs | 17 +++++++++ .../Helpers/ParallelRowIteratorTests.cs | 36 +++++++++++++++++++ tests/ImageSharp.Tests/TestImages.cs | 1 + .../Images/Input/Jpg/issues/Hang_C438A851.jpg | 3 ++ 6 files changed, 68 insertions(+), 6 deletions(-) create mode 100644 tests/Images/Input/Jpg/issues/Hang_C438A851.jpg diff --git a/src/ImageSharp/Advanced/ParallelRowIterator.cs b/src/ImageSharp/Advanced/ParallelRowIterator.cs index e787b7cfc5..06fbe731d1 100644 --- a/src/ImageSharp/Advanced/ParallelRowIterator.cs +++ b/src/ImageSharp/Advanced/ParallelRowIterator.cs @@ -52,7 +52,7 @@ public static void IterateRows( int width = rectangle.Width; int height = rectangle.Height; - int maxSteps = DivideCeil(width * height, parallelSettings.MinimumPixelsProcessedPerTask); + int maxSteps = DivideCeil(width * (long)height, parallelSettings.MinimumPixelsProcessedPerTask); int numOfSteps = Math.Min(parallelSettings.MaxDegreeOfParallelism, maxSteps); // Avoid TPL overhead in this trivial case: @@ -117,7 +117,7 @@ public static void IterateRows( int width = rectangle.Width; int height = rectangle.Height; - int maxSteps = DivideCeil(width * height, parallelSettings.MinimumPixelsProcessedPerTask); + int maxSteps = DivideCeil(width * (long)height, parallelSettings.MinimumPixelsProcessedPerTask); int numOfSteps = Math.Min(parallelSettings.MaxDegreeOfParallelism, maxSteps); MemoryAllocator allocator = parallelSettings.MemoryAllocator; @@ -181,7 +181,7 @@ public static void IterateRowIntervals( int width = rectangle.Width; int height = rectangle.Height; - int maxSteps = DivideCeil(width * height, parallelSettings.MinimumPixelsProcessedPerTask); + int maxSteps = DivideCeil(width * (long)height, parallelSettings.MinimumPixelsProcessedPerTask); int numOfSteps = Math.Min(parallelSettings.MaxDegreeOfParallelism, maxSteps); // Avoid TPL overhead in this trivial case: @@ -243,7 +243,7 @@ public static void IterateRowIntervals( int width = rectangle.Width; int height = rectangle.Height; - int maxSteps = DivideCeil(width * height, parallelSettings.MinimumPixelsProcessedPerTask); + int maxSteps = DivideCeil(width * (long)height, parallelSettings.MinimumPixelsProcessedPerTask); int numOfSteps = Math.Min(parallelSettings.MaxDegreeOfParallelism, maxSteps); MemoryAllocator allocator = parallelSettings.MemoryAllocator; @@ -270,7 +270,7 @@ public static void IterateRowIntervals( } [MethodImpl(InliningOptions.ShortMethod)] - private static int DivideCeil(int dividend, int divisor) => 1 + ((dividend - 1) / divisor); + private static int DivideCeil(long dividend, int divisor) => (int)Math.Min(1 + ((dividend - 1) / divisor), int.MaxValue); private static void ValidateRectangle(Rectangle rectangle) { diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanBuffer.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanBuffer.cs index 3664cb4eb3..39e606fd39 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanBuffer.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanBuffer.cs @@ -211,7 +211,12 @@ public bool FindNextMarker() private int ReadStream() { int value = this.badData ? 0 : this.stream.ReadByte(); - if (value == -1) + + // We've encountered the end of the file stream which means there's no EOI marker or the marker has been read + // during decoding of the SOS marker. + // When reading individual bits 'badData' simply means we have hit a marker, When data is '0' and the stream is exhausted + // we know we have hit the EOI and completed decoding the scan buffer. + if (value == -1 || (this.badData && this.data == 0 && this.stream.Position >= this.stream.Length)) { // We've encountered the end of the file stream which means there's no EOI marker // in the image or the SOS marker has the wrong dimensions set. diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index d12c840a1b..708e859eed 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -261,5 +261,22 @@ public void ValidateProgressivePdfJsOutput( this.Output.WriteLine($"Difference for PORT: {portReport.DifferencePercentageString}"); } } + + [Theory] + [WithFile(TestImages.Jpeg.Issues.HangBadScan, PixelTypes.L8)] + public void DecodeHang(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + if (TestEnvironment.IsWindows && + TestEnvironment.RunsOnCI) + { + // Windows CI runs consistently fail with OOM. + return; + } + + using Image image = provider.GetImage(JpegDecoder); + Assert.Equal(65503, image.Width); + Assert.Equal(65503, image.Height); + } } } diff --git a/tests/ImageSharp.Tests/Helpers/ParallelRowIteratorTests.cs b/tests/ImageSharp.Tests/Helpers/ParallelRowIteratorTests.cs index f46c9519ca..7585998a64 100644 --- a/tests/ImageSharp.Tests/Helpers/ParallelRowIteratorTests.cs +++ b/tests/ImageSharp.Tests/Helpers/ParallelRowIteratorTests.cs @@ -4,6 +4,7 @@ using System; using System.Linq; using System.Numerics; +using System.Runtime.CompilerServices; using System.Threading; using SixLabors.ImageSharp.Advanced; using SixLabors.ImageSharp.Memory; @@ -410,6 +411,41 @@ void RowAction(RowInterval rows, Span memory) Assert.Contains(width <= 0 ? "Width" : "Height", ex.Message); } + [Fact] + public void CanIterateWithoutIntOverflow() + { + ParallelExecutionSettings parallelSettings = ParallelExecutionSettings.FromConfiguration(Configuration.Default); + const int max = 100_000; + + Rectangle rect = new(0, 0, max, max); + int intervalMaxY = 0; + void RowAction(RowInterval rows, Span memory) => intervalMaxY = Math.Max(rows.Max, intervalMaxY); + + TestRowOperation operation = new(0); + TestRowIntervalOperation intervalOperation = new(RowAction); + + ParallelRowIterator.IterateRows(Configuration.Default, rect, in operation); + Assert.Equal(max - 1, operation.MaxY.Value); + + ParallelRowIterator.IterateRowIntervals, Rgba32>(rect, in parallelSettings, in intervalOperation); + Assert.Equal(max, intervalMaxY); + } + + private readonly struct TestRowOperation : IRowOperation + { + public TestRowOperation(int _) => this.MaxY = new StrongBox(); + + public StrongBox MaxY { get; } + + public void Invoke(int y) + { + lock (this.MaxY) + { + this.MaxY.Value = Math.Max(y, this.MaxY.Value); + } + } + } + private readonly struct TestRowIntervalOperation : IRowIntervalOperation { private readonly Action action; diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 13f965a592..874a980881 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -266,6 +266,7 @@ public static class Issues public const string ExifNullArrayTag = "Jpg/issues/issue-2056-exif-null-array.jpg"; public const string ValidExifArgumentNullExceptionOnEncode = "Jpg/issues/Issue2087-exif-null-reference-on-encode.jpg"; public const string Issue2133DeduceColorSpace = "Jpg/issues/Issue2133.jpg"; + public const string HangBadScan = "Jpg/issues/Hang_C438A851.jpg"; public static class Fuzz { diff --git a/tests/Images/Input/Jpg/issues/Hang_C438A851.jpg b/tests/Images/Input/Jpg/issues/Hang_C438A851.jpg new file mode 100644 index 0000000000..97ab9ad0fb --- /dev/null +++ b/tests/Images/Input/Jpg/issues/Hang_C438A851.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:580760756f2e7e3ed0752a4ec53d6b6786a4f005606f3a50878f732b3b2a1bcb +size 413