Merge pull request #699 from SixLabors/js/fix-exif-overflow

Fix EXIF overflow and Jpeg decoding

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)
                 {
@@ -914,9 +917,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg
         /// <param name="values">The values</param>
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private void BuildHuffmanTable(HuffmanTables tables, int index, ReadOnlySpan<byte> codeLengths, ReadOnlySpan<byte> values)
-        {
-            tables[index] = new HuffmanTable(this.configuration.MemoryAllocator, codeLengths, values);
-        }
+            => tables[index] = new HuffmanTable(this.configuration.MemoryAllocator, codeLengths, values);
 
         /// <summary>
         /// Reads a <see cref="ushort"/> from the stream advancing it by two bytes

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
         /// </summary>
         /// <param name="values">The values.</param>
         /// <param name="index">The index.</param>
-        private void AddValues(List<ExifValue> values, int index)
+        private void AddValues(List<ExifValue> 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<ExifValue>();
             this.AddValues(values, offset);
@@ -515,10 +520,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif
             return new Rational(numerator, denominator, false);
         }
 
-        private sbyte ConvertToSignedByte(ReadOnlySpan<byte> buffer)
-        {
-            return unchecked((sbyte)buffer[0]);
-        }
+        private sbyte ConvertToSignedByte(ReadOnlySpan<byte> buffer) => unchecked((sbyte)buffer[0]);
 
         private int ConvertToInt32(ReadOnlySpan<byte> buffer) // SignedLong in Exif Specification
         {

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 =

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

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();

tests/Images/External

@@ -1 +1 @@
-Subproject commit fcf311bf15bea061e552e4cc357cafe2d4f4bd70
+Subproject commit 6abc3bc0ac253a24c9e88e68d7b7d853350a85da