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 @@ 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 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 @@ 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 4be6731cc3..4b68c39ea9 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 b6c7e76268..0292fbcab4 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/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index ffaa9d567f..f81cbca211 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -1206,16 +1206,16 @@ namespace SixLabors.ImageSharp.Formats.Png PngChunkType type = this.ReadChunkType(); - // NOTE: Reading the Data chunk is the responsible of the caller // If we're reading color metadata only we're only interested in the IHDR and tRNS chunks. // We can skip all other chunk data in the stream for better performance. - if (type == PngChunkType.Data || (this.colorMetadataOnly && type != PngChunkType.Header && type != PngChunkType.Transparency)) + if (this.colorMetadataOnly && type != PngChunkType.Header && type != PngChunkType.Transparency) { chunk = new PngChunk(length, type); return true; } + long pos = this.currentStream.Position; chunk = new PngChunk( length: length, type: type, @@ -1223,6 +1223,13 @@ namespace SixLabors.ImageSharp.Formats.Png this.ValidateChunk(chunk); + // Restore the stream position for IDAT chunks, because it will be decoded later and + // was only read to verifying the CRC is correct. + if (type == PngChunkType.Data) + { + this.currentStream.Position = pos; + } + return true; } diff --git a/tests/ImageSharp.Tests/Common/NumericsTests.cs b/tests/ImageSharp.Tests/Common/NumericsTests.cs index 62819af493..a829974778 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 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 @@ 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 = diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index d63dad8d4b..c29f8c5891 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -255,7 +255,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png public void Decode_MissingDataChunk_ThrowsException(TestImageProvider provider) where TPixel : unmanaged, IPixel { - System.Exception ex = Record.Exception( + Exception ex = Record.Exception( () => { using Image image = provider.GetImage(PngDecoder); @@ -271,7 +271,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png public void Decode_InvalidBitDepth_ThrowsException(TestImageProvider provider) where TPixel : unmanaged, IPixel { - System.Exception ex = Record.Exception( + Exception ex = Record.Exception( () => { using Image image = provider.GetImage(PngDecoder); @@ -287,7 +287,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png public void Decode_InvalidColorType_ThrowsException(TestImageProvider provider) where TPixel : unmanaged, IPixel { - System.Exception ex = Record.Exception( + Exception ex = Record.Exception( () => { using Image image = provider.GetImage(PngDecoder); @@ -297,13 +297,27 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png Assert.Contains("Invalid or unsupported color type", ex.Message); } + [Theory] + [WithFile(TestImages.Png.Bad.WrongCrcDataChunk, PixelTypes.Rgba32)] + public void Decode_InvalidDataChunkCrc_ThrowsException(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + InvalidImageContentException ex = Assert.Throws( + () => + { + using Image image = provider.GetImage(PngDecoder); + }); + Assert.NotNull(ex); + Assert.Contains("CRC Error. PNG IDAT chunk is corrupt!", ex.Message); + } + // https://github.com/SixLabors/ImageSharp/issues/1014 [Theory] [WithFileCollection(nameof(TestImagesIssue1014), PixelTypes.Rgba32)] public void Issue1014_DataSplitOverMultipleIDatChunks(TestImageProvider provider) where TPixel : unmanaged, IPixel { - System.Exception ex = Record.Exception( + Exception ex = Record.Exception( () => { using Image image = provider.GetImage(PngDecoder); @@ -319,7 +333,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png public void Issue1177_CRC_Omitted(TestImageProvider provider) where TPixel : unmanaged, IPixel { - System.Exception ex = Record.Exception( + Exception ex = Record.Exception( () => { using Image image = provider.GetImage(PngDecoder); @@ -335,7 +349,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png public void Issue1127(TestImageProvider provider) where TPixel : unmanaged, IPixel { - System.Exception ex = Record.Exception( + Exception ex = Record.Exception( () => { using Image image = provider.GetImage(PngDecoder); @@ -351,7 +365,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png public void Issue1047(TestImageProvider provider) where TPixel : unmanaged, IPixel { - System.Exception ex = Record.Exception( + Exception ex = Record.Exception( () => { using Image image = provider.GetImage(PngDecoder); @@ -372,7 +386,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png public void Issue1765(TestImageProvider provider) where TPixel : unmanaged, IPixel { - System.Exception ex = Record.Exception( + Exception ex = Record.Exception( () => { using Image image = provider.GetImage(PngDecoder); @@ -388,7 +402,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png public void Issue410_MalformedApplePng(TestImageProvider provider) where TPixel : unmanaged, IPixel { - System.Exception ex = Record.Exception( + Exception ex = Record.Exception( () => { using Image image = provider.GetImage(PngDecoder); diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 898c031c23..3259357692 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -120,6 +120,7 @@ namespace SixLabors.ImageSharp.Tests public static class Bad { public const string MissingDataChunk = "Png/xdtn0g01.png"; + public const string WrongCrcDataChunk = "Png/xcsn0g01.png"; public const string CorruptedChunk = "Png/big-corrupted-chunk.png"; // Zlib errors. diff --git a/tests/Images/Input/Png/xcsn0g01.png b/tests/Images/Input/Png/xcsn0g01.png new file mode 100644 index 0000000000..6908a107c4 --- /dev/null +++ b/tests/Images/Input/Png/xcsn0g01.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:71e4b2826f61556eda39f3a93c8769b14d3ac90f135177b9373061199dbef39a +size 164