From e045fbb023b561f9b327e434e7aae319772372b6 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 17 Dec 2021 13:53:46 +0300 Subject: [PATCH 1/4] Sampling factor validation --- .../Jpeg/Components/Decoder/JpegComponent.cs | 11 ------- .../Formats/Jpeg/JpegDecoderCore.cs | 29 +++++++++++++++++-- .../Formats/Jpg/JpegDecoderTests.Images.cs | 6 ++-- 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponent.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponent.cs index 4da422e7f3..3804e1c6c0 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponent.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponent.cs @@ -19,21 +19,10 @@ public JpegComponent(MemoryAllocator memoryAllocator, JpegFrame frame, byte id, 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 4be6731cc3..bd221a357a 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -1022,10 +1022,26 @@ private void ProcessStartOfFrameMarker(BufferedReadStream stream, int remaining, 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 (h is < 1 or > 4) + { + JpegThrowHelper.ThrowInvalidImageContentException($"Bad horizontal sampling factor: {h}"); + } + + // Validate: 1-4 range + if (v is < 1 or > 4) + { + JpegThrowHelper.ThrowInvalidImageContentException($"Bad vertical sampling factor: {v}"); + } + if (maxH < h) { maxH = h; @@ -1036,10 +1052,19 @@ private void ProcessStartOfFrameMarker(BufferedReadStream stream, int remaining, 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/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs index ef817154d6..d82359e61a 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs @@ -40,9 +40,6 @@ public partial class JpegDecoderTests // 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 @@ public partial class JpegDecoderTests 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 = From 1f9ef3e926372325ebc8d342729f03cb782e880a Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Sat, 18 Dec 2021 11:13:39 +0300 Subject: [PATCH 2/4] Removed unused methods, added new throw helper method --- .../Formats/Jpeg/JpegDecoderCore.cs | 4 ++-- .../Formats/Jpeg/JpegThrowHelper.cs | 20 +++---------------- 2 files changed, 5 insertions(+), 19 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index bd221a357a..4543153def 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -1033,13 +1033,13 @@ private void ProcessStartOfFrameMarker(BufferedReadStream stream, int remaining, // Validate: 1-4 range if (h is < 1 or > 4) { - JpegThrowHelper.ThrowInvalidImageContentException($"Bad horizontal sampling factor: {h}"); + JpegThrowHelper.ThrowBadSampling(h); } // Validate: 1-4 range if (v is < 1 or > 4) { - JpegThrowHelper.ThrowInvalidImageContentException($"Bad vertical sampling factor: {v}"); + JpegThrowHelper.ThrowBadSampling(v); } if (maxH < h) diff --git a/src/ImageSharp/Formats/Jpeg/JpegThrowHelper.cs b/src/ImageSharp/Formats/Jpeg/JpegThrowHelper.cs index b6c7e76268..0292fbcab4 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegThrowHelper.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegThrowHelper.cs @@ -22,23 +22,6 @@ internal static class JpegThrowHelper [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 @@ public static void ThrowNotImplementedException(string errorMessage) [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}."); From f3b8e5cd8d1e00492367b40498de1a67f6da2375 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Sat, 18 Dec 2021 15:42:13 +0300 Subject: [PATCH 3/4] gfoidl IsOutOfRange --- src/ImageSharp/Common/Helpers/Numerics.cs | 9 +++++++++ .../Formats/Jpeg/JpegDecoderCore.cs | 4 ++-- .../ImageSharp.Tests/Common/NumericsTests.cs | 20 +++++++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Common/Helpers/Numerics.cs b/src/ImageSharp/Common/Helpers/Numerics.cs index 513db0c448..8f82476e13 100644 --- a/src/ImageSharp/Common/Helpers/Numerics.cs +++ b/src/ImageSharp/Common/Helpers/Numerics.cs @@ -963,5 +963,14 @@ public static uint RotateRight(uint value, int offset) 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/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 4543153def..4b68c39ea9 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -1031,13 +1031,13 @@ private void ProcessStartOfFrameMarker(BufferedReadStream stream, int remaining, int v = hv & 15; // Validate: 1-4 range - if (h is < 1 or > 4) + if (Numerics.IsOutOfRange(h, 1, 4)) { JpegThrowHelper.ThrowBadSampling(h); } // Validate: 1-4 range - if (v is < 1 or > 4) + if (Numerics.IsOutOfRange(v, 1, 4)) { JpegThrowHelper.ThrowBadSampling(v); } diff --git a/tests/ImageSharp.Tests/Common/NumericsTests.cs b/tests/ImageSharp.Tests/Common/NumericsTests.cs index 62819af493..2a43b06a92 100644 --- a/tests/ImageSharp.Tests/Common/NumericsTests.cs +++ b/tests/ImageSharp.Tests/Common/NumericsTests.cs @@ -97,5 +97,25 @@ public void DivideCeil_RandomValues(int seed, int count) 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] + [InlineData(1, 100)] + public void IsOutOfRange(int seed, int count) + { + var rng = new Random(seed); + for (int i = 0; i < count; i++) + { + int value = rng.Next(); + int min = rng.Next(); + int max = rng.Next(min, int.MaxValue); + + bool expected = IsOutOfRange_ReferenceImplementation(value, min, max); + bool actual = Numerics.IsOutOfRange(value, min, max); + + Assert.True(expected == actual, $"IsOutOfRange({value}, {min}, {max})"); + } + } } } From 7ced49747e84744c93b6dca6138257e0d9395a11 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Sat, 18 Dec 2021 23:41:55 +0300 Subject: [PATCH 4/4] Fixed tests --- .../ImageSharp.Tests/Common/NumericsTests.cs | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/tests/ImageSharp.Tests/Common/NumericsTests.cs b/tests/ImageSharp.Tests/Common/NumericsTests.cs index 2a43b06a92..a829974778 100644 --- a/tests/ImageSharp.Tests/Common/NumericsTests.cs +++ b/tests/ImageSharp.Tests/Common/NumericsTests.cs @@ -16,6 +16,8 @@ public NumericsTests(ITestOutputHelper output) 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; @@ -101,21 +103,16 @@ public void DivideCeil_RandomValues(int seed, int count) private static bool IsOutOfRange_ReferenceImplementation(int value, int min, int max) => value < min || value > max; [Theory] - [InlineData(1, 100)] - public void IsOutOfRange(int seed, int count) + [MemberData(nameof(IsOutOfRangeTestData))] + public void IsOutOfRange(int value) { - var rng = new Random(seed); - for (int i = 0; i < count; i++) - { - int value = rng.Next(); - int min = rng.Next(); - int max = rng.Next(min, int.MaxValue); + const int min = 7; + const int max = 92; - bool expected = IsOutOfRange_ReferenceImplementation(value, min, max); - bool actual = Numerics.IsOutOfRange(value, min, max); + bool expected = IsOutOfRange_ReferenceImplementation(value, min, max); + bool actual = Numerics.IsOutOfRange(value, min, max); - Assert.True(expected == actual, $"IsOutOfRange({value}, {min}, {max})"); - } + Assert.True(expected == actual, $"IsOutOfRange({value}, {min}, {max})"); } } }