diff --git a/src/ImageSharp/Common/Helpers/Numerics.cs b/src/ImageSharp/Common/Helpers/Numerics.cs index 513db0c44..8f82476e1 100644 --- a/src/ImageSharp/Common/Helpers/Numerics.cs +++ b/src/ImageSharp/Common/Helpers/Numerics.cs @@ -963,5 +963,14 @@ namespace SixLabors.ImageSharp public static uint RotateRightSoftwareFallback(uint value, int offset) => (value >> offset) | (value << (32 - offset)); #endif + + /// + /// Tells whether input value is outside of the given range. + /// + /// Value. + /// Mininum value, inclusive. + /// Maximum value, inclusive. + public static bool IsOutOfRange(int value, int min, int max) + => (uint)(value - min) > (uint)(max - min); } } diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponent.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponent.cs index 4da422e7f..3804e1c6c 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponent.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponent.cs @@ -19,21 +19,10 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder this.Frame = frame; this.Id = id; - // Validate sampling factors. - if (horizontalFactor == 0 || verticalFactor == 0) - { - JpegThrowHelper.ThrowBadSampling(); - } - this.HorizontalSamplingFactor = horizontalFactor; this.VerticalSamplingFactor = verticalFactor; this.SamplingFactors = new Size(this.HorizontalSamplingFactor, this.VerticalSamplingFactor); - if (quantizationTableIndex > 3) - { - JpegThrowHelper.ThrowBadQuantizationTableIndex(quantizationTableIndex); - } - this.QuantizationTableIndex = quantizationTableIndex; this.Index = index; } diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 4be6731cc..4b68c39ea 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -1022,10 +1022,26 @@ namespace SixLabors.ImageSharp.Formats.Jpeg int index = 0; for (int i = 0; i < componentCount; i++) { + // 1 byte: component identifier + byte componentId = this.temp[index]; + + // 1 byte: component sampling factors byte hv = this.temp[index + 1]; int h = (hv >> 4) & 15; int v = hv & 15; + // Validate: 1-4 range + if (Numerics.IsOutOfRange(h, 1, 4)) + { + JpegThrowHelper.ThrowBadSampling(h); + } + + // Validate: 1-4 range + if (Numerics.IsOutOfRange(v, 1, 4)) + { + JpegThrowHelper.ThrowBadSampling(v); + } + if (maxH < h) { maxH = h; @@ -1036,10 +1052,19 @@ namespace SixLabors.ImageSharp.Formats.Jpeg maxV = v; } - var component = new JpegComponent(this.Configuration.MemoryAllocator, this.Frame, this.temp[index], h, v, this.temp[index + 2], i); + // 1 byte: quantization table destination selector + byte quantTableIndex = this.temp[index + 2]; + + // Validate: 0-3 range + if (quantTableIndex > 3) + { + JpegThrowHelper.ThrowBadQuantizationTableIndex(quantTableIndex); + } + + var component = new JpegComponent(this.Configuration.MemoryAllocator, this.Frame, componentId, h, v, quantTableIndex, i); this.Frame.Components[i] = component; - this.Frame.ComponentIds[i] = component.Id; + this.Frame.ComponentIds[i] = componentId; index += componentBytes; } diff --git a/src/ImageSharp/Formats/Jpeg/JpegThrowHelper.cs b/src/ImageSharp/Formats/Jpeg/JpegThrowHelper.cs index b6c7e7626..0292fbcab 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegThrowHelper.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegThrowHelper.cs @@ -22,23 +22,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg [MethodImpl(InliningOptions.ColdPath)] public static void ThrowInvalidImageContentException(string errorMessage) => throw new InvalidImageContentException(errorMessage); - /// - /// Cold path optimization for throwing 's. - /// - /// The error message for the exception. - /// The exception that is the cause of the current exception, or a null reference - /// if no inner exception is specified. - [MethodImpl(InliningOptions.ColdPath)] - public static void ThrowInvalidImageContentException(string errorMessage, Exception innerException) => throw new InvalidImageContentException(errorMessage, innerException); - - /// - /// Cold path optimization for throwing 's - /// - /// The error message for the exception. - [MethodImpl(InliningOptions.ColdPath)] - public static void ThrowNotImplementedException(string errorMessage) - => throw new NotImplementedException(errorMessage); - [MethodImpl(InliningOptions.ColdPath)] public static void ThrowBadMarker(string marker, int length) => throw new InvalidImageContentException($"Marker {marker} has bad length {length}."); @@ -51,6 +34,9 @@ namespace SixLabors.ImageSharp.Formats.Jpeg [MethodImpl(InliningOptions.ColdPath)] public static void ThrowBadSampling() => throw new InvalidImageContentException("Bad sampling factor."); + [MethodImpl(InliningOptions.ColdPath)] + public static void ThrowBadSampling(int factor) => throw new InvalidImageContentException($"Bad sampling factor: {factor}"); + [MethodImpl(InliningOptions.ColdPath)] public static void ThrowBadProgressiveScan(int ss, int se, int ah, int al) => throw new InvalidImageContentException($"Invalid progressive parameters Ss={ss} Se={se} Ah={ah} Al={al}."); diff --git a/tests/ImageSharp.Tests/Common/NumericsTests.cs b/tests/ImageSharp.Tests/Common/NumericsTests.cs index 62819af49..a82997477 100644 --- a/tests/ImageSharp.Tests/Common/NumericsTests.cs +++ b/tests/ImageSharp.Tests/Common/NumericsTests.cs @@ -16,6 +16,8 @@ namespace SixLabors.ImageSharp.Tests.Common this.Output = output; } + public static TheoryData IsOutOfRangeTestData = new() { int.MinValue, -1, 0, 1, 6, 7, 8, 91, 92, 93, int.MaxValue }; + private static int Log2_ReferenceImplementation(uint value) { int n = 0; @@ -97,5 +99,20 @@ namespace SixLabors.ImageSharp.Tests.Common Assert.True(expected == actual, $"Expected: {expected}\nActual: {actual}\n{value} / {divisor} = {expected}"); } } + + private static bool IsOutOfRange_ReferenceImplementation(int value, int min, int max) => value < min || value > max; + + [Theory] + [MemberData(nameof(IsOutOfRangeTestData))] + public void IsOutOfRange(int value) + { + const int min = 7; + const int max = 92; + + bool expected = IsOutOfRange_ReferenceImplementation(value, min, max); + bool actual = Numerics.IsOutOfRange(value, min, max); + + Assert.True(expected == actual, $"IsOutOfRange({value}, {min}, {max})"); + } } } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs index ef817154d..d82359e61 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs @@ -40,9 +40,6 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg // LibJpeg can open this despite incorrect colorspace metadata. TestImages.Jpeg.Issues.IncorrectColorspace855, - // LibJpeg can open this despite the invalid subsampling units. - TestImages.Jpeg.Issues.Fuzz.IndexOutOfRangeException824C, - // High depth images TestImages.Jpeg.Baseline.Testorig12bit, }; @@ -90,7 +87,8 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg TestImages.Jpeg.Issues.Fuzz.AccessViolationException827, TestImages.Jpeg.Issues.Fuzz.ExecutionEngineException839, TestImages.Jpeg.Issues.Fuzz.IndexOutOfRangeException1693A, - TestImages.Jpeg.Issues.Fuzz.IndexOutOfRangeException1693B + TestImages.Jpeg.Issues.Fuzz.IndexOutOfRangeException1693B, + TestImages.Jpeg.Issues.Fuzz.IndexOutOfRangeException824C, }; private static readonly Dictionary CustomToleranceValues =