Skip to content
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.

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

Already on GitHub? Sign in to your account

Backport - Handle EOF in Jpeg bit reader when data is bad to prevent DOS attack. #2524

Merged
merged 1 commit into from
Aug 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/ImageSharp/Advanced/ParallelRowIterator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public static void IterateRows<T>(Configuration configuration, Rectangle rectang
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:
Expand Down Expand Up @@ -117,7 +117,7 @@ public static void IterateRows<T>(Configuration configuration, Rectangle rectang
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;

Expand Down Expand Up @@ -181,7 +181,7 @@ public static void IterateRowIntervals<T>(Configuration configuration, Rectangle
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:
Expand Down Expand Up @@ -243,7 +243,7 @@ public static void IterateRowIntervals<T>(Configuration configuration, Rectangle
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;

Expand All @@ -270,7 +270,7 @@ public static void IterateRowIntervals<T>(Configuration configuration, Rectangle
}

[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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
17 changes: 17 additions & 0 deletions tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -261,5 +261,22 @@ public void Issue2133_DeduceColorSpace<TPixel>(TestImageProvider<TPixel> provide
this.Output.WriteLine($"Difference for PORT: {portReport.DifferencePercentageString}");
}
}

[Theory]
[WithFile(TestImages.Jpeg.Issues.HangBadScan, PixelTypes.L8)]
public void DecodeHang<TPixel>(TestImageProvider<TPixel> provider)
where TPixel : unmanaged, IPixel<TPixel>
{
if (TestEnvironment.IsWindows &&
TestEnvironment.RunsOnCI)
{
// Windows CI runs consistently fail with OOM.
return;
}

using Image<TPixel> image = provider.GetImage(JpegDecoder);
Assert.Equal(65503, image.Width);
Assert.Equal(65503, image.Height);
}
}
}
36 changes: 36 additions & 0 deletions tests/ImageSharp.Tests/Helpers/ParallelRowIteratorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -410,6 +411,41 @@ void RowAction(RowInterval rows, Span<Rgba32> 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<Rgba32> memory) => intervalMaxY = Math.Max(rows.Max, intervalMaxY);

TestRowOperation operation = new(0);
TestRowIntervalOperation<Rgba32> intervalOperation = new(RowAction);

ParallelRowIterator.IterateRows(Configuration.Default, rect, in operation);
Assert.Equal(max - 1, operation.MaxY.Value);

ParallelRowIterator.IterateRowIntervals<TestRowIntervalOperation<Rgba32>, Rgba32>(rect, in parallelSettings, in intervalOperation);
Assert.Equal(max, intervalMaxY);
}

private readonly struct TestRowOperation : IRowOperation
{
public TestRowOperation(int _) => this.MaxY = new StrongBox<int>();

public StrongBox<int> 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<RowInterval> action;
Expand Down
1 change: 1 addition & 0 deletions tests/ImageSharp.Tests/TestImages.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
3 changes: 3 additions & 0 deletions tests/Images/Input/Jpg/issues/Hang_C438A851.jpg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.