From cf2e641000c832f3bdb0ff55b6e19ba6798a8f4a Mon Sep 17 00:00:00 2001 From: Dirk Lemstra Date: Thu, 6 Sep 2018 22:27:28 +0200 Subject: [PATCH 1/3] Allow several invalid data types when reading the exif resolution. --- .../Formats/Jpeg/JpegDecoderCore.cs | 29 ++++++++++++++----- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 7561afa1e..cd960a241 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -463,13 +463,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg } else if (this.isExif) { - double horizontalValue = this.MetaData.ExifProfile.TryGetValue(ExifTag.XResolution, out ExifValue horizontalTag) - ? ((Rational)horizontalTag.Value).ToDouble() - : 0; - - double verticalValue = this.MetaData.ExifProfile.TryGetValue(ExifTag.YResolution, out ExifValue verticalTag) - ? ((Rational)verticalTag.Value).ToDouble() - : 0; + double horizontalValue = this.GetExifResolutionValue(ExifTag.XResolution); + double verticalValue = this.GetExifResolutionValue(ExifTag.YResolution); if (horizontalValue > 0 && verticalValue > 0) { @@ -480,6 +475,26 @@ namespace SixLabors.ImageSharp.Formats.Jpeg } } + private double GetExifResolutionValue(ExifTag tag) + { + if (!this.MetaData.ExifProfile.TryGetValue(tag, out ExifValue exifValue)) + { + return 0; + } + + switch (exifValue.DataType) + { + case ExifDataType.Rational: + return ((Rational)exifValue.Value).ToDouble(); + case ExifDataType.Long: + return (uint)exifValue.Value; + case ExifDataType.DoubleFloat: + return (double)exifValue.Value; + default: + return 0; + } + } + /// /// Extends the profile with additional data. /// From bd4d5ba32d6c62109478fb961844b1a63c1b136a Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 6 Sep 2018 21:47:49 +0100 Subject: [PATCH 2/3] Fix EXIF overflow and Jpeg decoding --- .../Formats/Jpeg/JpegDecoderCore.cs | 11 +++++---- .../MetaData/Profiles/Exif/ExifReader.cs | 24 ++++++++++--------- .../Formats/Jpg/JpegDecoderTests.Images.cs | 6 ++++- .../Formats/Jpg/JpegDecoderTests.cs | 2 ++ tests/ImageSharp.Tests/TestImages.cs | 3 +++ tests/Images/External | 2 +- .../Issue694-Decode-Exif-OutOfRange.jpg | 3 +++ .../Input/Jpg/issues/Issue695-Invalid-EOI.jpg | 3 +++ .../Issue696-Resize-Exif-OutOfRange.jpg | 3 +++ 9 files changed, 39 insertions(+), 18 deletions(-) create mode 100644 tests/Images/Input/Jpg/issues/Issue694-Decode-Exif-OutOfRange.jpg create mode 100644 tests/Images/Input/Jpg/issues/Issue695-Invalid-EOI.jpg create mode 100644 tests/Images/Input/Jpg/issues/Issue696-Resize-Exif-OutOfRange.jpg diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 7561afa1e..057bd74bb 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -268,7 +268,10 @@ namespace SixLabors.ImageSharp.Formats.Jpeg this.fastACTables = new FastACTables(this.configuration.MemoryAllocator); } - while (fileMarker.Marker != JpegConstants.Markers.EOI) + // Break only when we discover a valid EOI marker. + // https://github.com/SixLabors/ImageSharp/issues/695 + while (fileMarker.Marker != JpegConstants.Markers.EOI + || (fileMarker.Marker == JpegConstants.Markers.EOI && fileMarker.Invalid)) { if (!fileMarker.Invalid) { @@ -898,10 +901,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg /// The codelengths /// The values [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void BuildHuffmanTable(HuffmanTables tables, int index, ReadOnlySpan codeLengths, ReadOnlySpan values) - { - tables[index] = new HuffmanTable(this.configuration.MemoryAllocator, codeLengths, values); - } + private void BuildHuffmanTable(HuffmanTables tables, int index, ReadOnlySpan codeLengths, ReadOnlySpan values) + => tables[index] = new HuffmanTable(this.configuration.MemoryAllocator, codeLengths, values); /// /// Reads a from the stream advancing it by two bytes diff --git a/src/ImageSharp/MetaData/Profiles/Exif/ExifReader.cs b/src/ImageSharp/MetaData/Profiles/Exif/ExifReader.cs index 72db6305d..549cb3fe0 100644 --- a/src/ImageSharp/MetaData/Profiles/Exif/ExifReader.cs +++ b/src/ImageSharp/MetaData/Profiles/Exif/ExifReader.cs @@ -88,19 +88,19 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif } uint ifdOffset = this.ReadUInt32(); - this.AddValues(values, (int)ifdOffset); + this.AddValues(values, ifdOffset); uint thumbnailOffset = this.ReadUInt32(); - this.GetThumbnail((int)thumbnailOffset); + this.GetThumbnail(thumbnailOffset); if (this.exifOffset != 0) { - this.AddValues(values, (int)this.exifOffset); + this.AddValues(values, this.exifOffset); } if (this.gpsOffset != 0) { - this.AddValues(values, (int)this.gpsOffset); + this.AddValues(values, this.gpsOffset); } return values; @@ -153,9 +153,14 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif /// /// The values. /// The index. - private void AddValues(List values, int index) + private void AddValues(List values, uint index) { - this.position = index; + if (index > (uint)this.exifData.Length) + { + return; + } + + this.position = (int)index; int count = this.ReadUInt16(); for (int i = 0; i < count; i++) @@ -431,7 +436,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif return null; } - private void GetThumbnail(int offset) + private void GetThumbnail(uint offset) { var values = new List(); this.AddValues(values, offset); @@ -515,10 +520,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif return new Rational(numerator, denominator, false); } - private sbyte ConvertToSignedByte(ReadOnlySpan buffer) - { - return unchecked((sbyte)buffer[0]); - } + private sbyte ConvertToSignedByte(ReadOnlySpan buffer) => unchecked((sbyte)buffer[0]); private int ConvertToInt32(ReadOnlySpan buffer) // SignedLong in Exif Specification { diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs index 3c98d5be7..a14c4735f 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs @@ -21,7 +21,11 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg TestImages.Jpeg.Baseline.Bad.BadEOF, TestImages.Jpeg.Issues.MultiHuffmanBaseline394, TestImages.Jpeg.Baseline.MultiScanBaselineCMYK, - TestImages.Jpeg.Baseline.Bad.BadRST + TestImages.Jpeg.Baseline.Bad.BadRST, + TestImages.Jpeg.Issues.MultiHuffmanBaseline394, + TestImages.Jpeg.Issues.ExifDecodeOutOfRange694, + TestImages.Jpeg.Issues.InvalidEOI695, + TestImages.Jpeg.Issues.ExifResizeOutOfRange696 }; public static string[] ProgressiveTestJpegs = diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index 4ae955c32..5977e59cf 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -48,6 +48,8 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg TestImages.Jpeg.Issues.BadZigZagProgressive385, TestImages.Jpeg.Issues.NoEoiProgressive517, TestImages.Jpeg.Issues.BadRstProgressive518, + TestImages.Jpeg.Issues.InvalidEOI695, + TestImages.Jpeg.Issues.ExifResizeOutOfRange696 }; return !TestEnvironment.Is64BitProcess && largeImagesToSkipOn32Bit.Contains(provider.SourceFileOrDescription); diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 1ee3f9675..0ad3edda8 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -152,6 +152,9 @@ namespace SixLabors.ImageSharp.Tests public const string BadRstProgressive518 = "Jpg/issues/Issue518-Bad-RST-Progressive.jpg"; public const string InvalidCast520 = "Jpg/issues/Issue520-InvalidCast.jpg"; public const string DhtHasWrongLength624 = "Jpg/issues/Issue624-DhtHasWrongLength-Progressive-N.jpg"; + public const string ExifDecodeOutOfRange694 = "Jpg/issues/Issue694-Decode-Exif-OutOfRange.jpg"; + public const string InvalidEOI695 = "Jpg/issues/Issue695-Invalid-EOI.jpg"; + public const string ExifResizeOutOfRange696 = "Jpg/issues/Issue696-Resize-Exif-OutOfRange.jpg"; } public static readonly string[] All = Baseline.All.Concat(Progressive.All).ToArray(); diff --git a/tests/Images/External b/tests/Images/External index fcf311bf1..6abc3bc0a 160000 --- a/tests/Images/External +++ b/tests/Images/External @@ -1 +1 @@ -Subproject commit fcf311bf15bea061e552e4cc357cafe2d4f4bd70 +Subproject commit 6abc3bc0ac253a24c9e88e68d7b7d853350a85da diff --git a/tests/Images/Input/Jpg/issues/Issue694-Decode-Exif-OutOfRange.jpg b/tests/Images/Input/Jpg/issues/Issue694-Decode-Exif-OutOfRange.jpg new file mode 100644 index 000000000..b3a7ce356 --- /dev/null +++ b/tests/Images/Input/Jpg/issues/Issue694-Decode-Exif-OutOfRange.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:1b94a283fbe8927ab59745dd67d0b33e90a253e674e5fe4f0ad7594ff868cca2 +size 226421 diff --git a/tests/Images/Input/Jpg/issues/Issue695-Invalid-EOI.jpg b/tests/Images/Input/Jpg/issues/Issue695-Invalid-EOI.jpg new file mode 100644 index 000000000..489177987 --- /dev/null +++ b/tests/Images/Input/Jpg/issues/Issue695-Invalid-EOI.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:e748a684c318b7424dd77b008fe92e179201d0a55106021b453a3fd2a22e9ab6 +size 4805575 diff --git a/tests/Images/Input/Jpg/issues/Issue696-Resize-Exif-OutOfRange.jpg b/tests/Images/Input/Jpg/issues/Issue696-Resize-Exif-OutOfRange.jpg new file mode 100644 index 000000000..38c5c8d59 --- /dev/null +++ b/tests/Images/Input/Jpg/issues/Issue696-Resize-Exif-OutOfRange.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:bb22005e1db0f6da8f49ca57979f8b9aa5db7111b717a53e817e24c04283ab43 +size 3196058 From 1e3f4c5187719f4d28310bba6de0a545a3e6ee50 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 6 Sep 2018 22:21:16 +0100 Subject: [PATCH 3/3] Remove traililng whitespace --- src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 057bd74bb..2219edef5 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -901,7 +901,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg /// The codelengths /// The values [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void BuildHuffmanTable(HuffmanTables tables, int index, ReadOnlySpan codeLengths, ReadOnlySpan values) + private void BuildHuffmanTable(HuffmanTables tables, int index, ReadOnlySpan codeLengths, ReadOnlySpan values) => tables[index] = new HuffmanTable(this.configuration.MemoryAllocator, codeLengths, values); ///