Merge pull request #1899 from br3aker/dp/jpeg-sampling-factor-validation

Jpeg sampling factor validation #1894

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
+
+        /// <summary>
+        /// Tells whether input value is outside of the given range.
+        /// </summary>
+        /// <param name="value">Value.</param>
+        /// <param name="min">Mininum value, inclusive.</param>
+        /// <param name="max">Maximum value, inclusive.</param>
+        public static bool IsOutOfRange(int value, int min, int max)
+            => (uint)(value - min) > (uint)(max - min);
     }
 }

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;
         }

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;
             }

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);
 
-        /// <summary>
-        /// Cold path optimization for throwing <see cref="InvalidImageContentException"/>'s.
-        /// </summary>
-        /// <param name="errorMessage">The error message for the exception.</param>
-        /// <param name="innerException">The exception that is the cause of the current exception, or a null reference
-        /// if no inner exception is specified.</param>
-        [MethodImpl(InliningOptions.ColdPath)]
-        public static void ThrowInvalidImageContentException(string errorMessage, Exception innerException) => throw new InvalidImageContentException(errorMessage, innerException);
-
-        /// <summary>
-        /// Cold path optimization for throwing <see cref="NotImplementedException"/>'s
-        /// </summary>
-        /// <param name="errorMessage">The error message for the exception.</param>
-        [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}.");
 

tests/ImageSharp.Tests/Common/NumericsTests.cs

@@ -16,6 +16,8 @@ namespace SixLabors.ImageSharp.Tests.Common
             this.Output = output;
         }
 
+        public static TheoryData<int> 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})");
+        }
     }
 }

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<string, float> CustomToleranceValues =