Merge branch 'master' into feature/fix-576

src/ImageSharp/Formats/Jpeg/GolangPort/GolangJpegDecoderCore.cs

@@ -450,6 +450,11 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort
                     this.MetaData.VerticalResolution = verticalValue;
                 }
             }
+
+            if (this.MetaData.IccProfile?.CheckIsValid() == false)
+            {
+                this.MetaData.IccProfile = null;
+            }
         }
 
         /// <summary>

src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs

@@ -196,7 +196,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort
             where TPixel : struct, IPixel<TPixel>
         {
             this.ParseStream(stream);
-            this.AssignResolution();
+            this.InitDerivedMetaDataProperties();
             return this.PostProcessIntoImage<TPixel>();
         }
 
@@ -207,7 +207,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort
         public IImageInfo Identify(Stream stream)
         {
             this.ParseStream(stream, true);
-            this.AssignResolution();
+            this.InitDerivedMetaDataProperties();
             return new ImageInfo(new PixelTypeInfo(this.BitsPerPixel), this.ImageWidth, this.ImageHeight, this.MetaData);
         }
 
@@ -395,9 +395,9 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort
         }
 
         /// <summary>
-        /// Assigns the horizontal and vertical resolution to the image if it has a JFIF header or EXIF metadata.
+        /// Assigns derived metadata properties to <see cref="MetaData"/>, eg. horizontal and vertical resolution if it has a JFIF header.
         /// </summary>
-        private void AssignResolution()
+        private void InitDerivedMetaDataProperties()
         {
             if (this.jFif.XDensity > 0 && this.jFif.YDensity > 0)
             {
@@ -420,6 +420,11 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort
                     this.MetaData.VerticalResolution = verticalValue;
                 }
             }
+
+            if (this.MetaData.IccProfile?.CheckIsValid() == false)
+            {
+                this.MetaData.IccProfile = null;
+            }
         }
 
         /// <summary>

src/ImageSharp/MetaData/Profiles/ICC/DataReader/IccDataReader.cs

@@ -1,7 +1,6 @@
 // Copyright (c) Six Labors and contributors.
 // Licensed under the Apache License, Version 2.0.
 
-using System;
 using System.Text;
 
 namespace SixLabors.ImageSharp.MetaData.Profiles.Icc
@@ -11,7 +10,6 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Icc
     /// </summary>
     internal sealed partial class IccDataReader
     {
-        private static readonly bool IsLittleEndian = BitConverter.IsLittleEndian;
         private static readonly Encoding AsciiEncoding = Encoding.GetEncoding("ASCII");
 
         /// <summary>
@@ -34,6 +32,14 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Icc
             this.data = data;
         }
 
+        /// <summary>
+        /// Gets the length in bytes of the raw data
+        /// </summary>
+        public int DataLength
+        {
+            get { return this.data.Length; }
+        }
+
         /// <summary>
         /// Sets the reading position to the given value
         /// </summary>

src/ImageSharp/MetaData/Profiles/ICC/IccProfile.cs

@@ -52,17 +52,12 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Icc
         /// by making a copy from another ICC profile.
         /// </summary>
         /// <param name="other">The other ICC profile, where the clone should be made from.</param>
-        /// <exception cref="System.ArgumentNullException"><paramref name="other"/> is null.</exception>>
+        /// <exception cref="ArgumentNullException"><paramref name="other"/> is null.</exception>>
         public IccProfile(IccProfile other)
         {
             Guard.NotNull(other, nameof(other));
 
-            // TODO: Do we need to copy anything else?
-            if (other.data != null)
-            {
-                this.data = new byte[other.data.Length];
-                Buffer.BlockCopy(other.data, 0, this.data, 0, other.data.Length);
-            }
+            this.data = other.ToByteArray();
         }
 
         /// <summary>
@@ -108,7 +103,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Icc
 #if !NETSTANDARD1_1
 
         /// <summary>
-        /// Calculates the MD5 hash value of an ICC profile header
+        /// Calculates the MD5 hash value of an ICC profile
         /// </summary>
         /// <param name="data">The data of which to calculate the hash value</param>
         /// <returns>The calculated hash</returns>
@@ -117,22 +112,38 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Icc
             Guard.NotNull(data, nameof(data));
             Guard.IsTrue(data.Length >= 128, nameof(data), "Data length must be at least 128 to be a valid profile header");
 
-            byte[] header = new byte[128];
-            Buffer.BlockCopy(data, 0, header, 0, 128);
+            const int profileFlagPos = 44;
+            const int renderingIntentPos = 64;
+            const int profileIdPos = 84;
+
+            // need to copy some values because they need to be zero for the hashing
+            byte[] temp = new byte[24];
+            Buffer.BlockCopy(data, profileFlagPos, temp, 0, 4);
+            Buffer.BlockCopy(data, renderingIntentPos, temp, 4, 4);
+            Buffer.BlockCopy(data, profileIdPos, temp, 8, 16);
 
             using (var md5 = MD5.Create())
             {
-                // Zero out some values
-                Array.Clear(header, 44, 4);     // Profile flags
-                Array.Clear(header, 64, 4);     // Rendering Intent
-                Array.Clear(header, 84, 16);    // Profile ID
-
-                // Calculate hash
-                byte[] hash = md5.ComputeHash(data);
-
-                // Read values from hash
-                var reader = new IccDataReader(hash);
-                return reader.ReadProfileId();
+                try
+                {
+                    // Zero out some values
+                    Array.Clear(data, profileFlagPos, 4);
+                    Array.Clear(data, renderingIntentPos, 4);
+                    Array.Clear(data, profileIdPos, 16);
+
+                    // Calculate hash
+                    byte[] hash = md5.ComputeHash(data);
+
+                    // Read values from hash
+                    var reader = new IccDataReader(hash);
+                    return reader.ReadProfileId();
+                }
+                finally
+                {
+                    Buffer.BlockCopy(temp, 0, data, profileFlagPos, 4);
+                    Buffer.BlockCopy(temp, 4, data, renderingIntentPos, 4);
+                    Buffer.BlockCopy(temp, 8, data, profileIdPos, 16);
+                }
             }
         }
 
@@ -149,14 +160,37 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Icc
             Buffer.BlockCopy(bytes, 0, this.data, currentLength, bytes.Length);
         }
 
+        /// <summary>
+        /// Checks for signs of a corrupt profile.
+        /// </summary>
+        /// <remarks>This is not an absolute proof of validity but should weed out most corrupt data.</remarks>
+        /// <returns>True if the profile is valid; False otherwise</returns>
+        public bool CheckIsValid()
+        {
+            return Enum.IsDefined(typeof(IccColorSpaceType), this.Header.DataColorSpace) &&
+                   Enum.IsDefined(typeof(IccColorSpaceType), this.Header.ProfileConnectionSpace) &&
+                   Enum.IsDefined(typeof(IccRenderingIntent), this.Header.RenderingIntent) &&
+                   this.Header.Size >= 128 &&
+                   this.Header.Size < 50_000_000; // it's unlikely there is a profile bigger than 50MB
+        }
+
         /// <summary>
         /// Converts this instance to a byte array.
         /// </summary>
         /// <returns>The <see cref="T:byte[]"/></returns>
         public byte[] ToByteArray()
         {
-            var writer = new IccWriter();
-            return writer.Write(this);
+            if (this.data != null)
+            {
+                byte[] copy = new byte[this.data.Length];
+                Buffer.BlockCopy(this.data, 0, copy, 0, copy.Length);
+                return copy;
+            }
+            else
+            {
+                var writer = new IccWriter();
+                return writer.Write(this);
+            }
         }
 
         private void InitializeHeader()

src/ImageSharp/MetaData/Profiles/ICC/IccReader.cs

@@ -84,27 +84,36 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Icc
         private IccTagDataEntry[] ReadTagData(IccDataReader reader)
         {
             IccTagTableEntry[] tagTable = this.ReadTagTable(reader);
-            var entries = new IccTagDataEntry[tagTable.Length];
+            var entries = new List<IccTagDataEntry>(tagTable.Length);
             var store = new Dictionary<uint, IccTagDataEntry>();
-            for (int i = 0; i < tagTable.Length; i++)
+
+            foreach (IccTagTableEntry tag in tagTable)
             {
                 IccTagDataEntry entry;
-                uint offset = tagTable[i].Offset;
-                if (store.ContainsKey(offset))
+                if (store.ContainsKey(tag.Offset))
                 {
-                    entry = store[offset];
+                    entry = store[tag.Offset];
                 }
                 else
                 {
-                    entry = reader.ReadTagDataEntry(tagTable[i]);
-                    store.Add(offset, entry);
+                    try
+                    {
+                        entry = reader.ReadTagDataEntry(tag);
+                    }
+                    catch
+                    {
+                        // Ignore tags that could not be read
+                        continue;
+                    }
+
+                    store.Add(tag.Offset, entry);
                 }
 
-                entry.TagSignature = tagTable[i].Signature;
-                entries[i] = entry;
+                entry.TagSignature = tag.Signature;
+                entries.Add(entry);
             }
 
-            return entries;
+            return entries.ToArray();
         }
 
         private IccTagTableEntry[] ReadTagTable(IccDataReader reader)
@@ -112,17 +121,29 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Icc
             reader.SetIndex(128);   // An ICC header is 128 bytes long
 
             uint tagCount = reader.ReadUInt32();
-            var table = new IccTagTableEntry[tagCount];
 
+            // Prevent creating huge arrays because of corrupt profiles.
+            // A normal profile usually has 5-15 entries
+            if (tagCount > 100)
+            {
+                return new IccTagTableEntry[0];
+            }
+
+            var table = new List<IccTagTableEntry>((int)tagCount);
             for (int i = 0; i < tagCount; i++)
             {
                 uint tagSignature = reader.ReadUInt32();
                 uint tagOffset = reader.ReadUInt32();
                 uint tagSize = reader.ReadUInt32();
-                table[i] = new IccTagTableEntry((IccProfileTag)tagSignature, tagOffset, tagSize);
+
+                // Exclude entries that have nonsense values and could cause exceptions further on
+                if (tagOffset < reader.DataLength && tagSize < reader.DataLength - 128)
+                {
+                    table.Add(new IccTagTableEntry((IccProfileTag)tagSignature, tagOffset, tagSize));
+                }
             }
 
-            return table;
+            return table.ToArray();
         }
     }
 }

tests/ImageSharp.Tests/MetaData/Profiles/ICC/IccProfileTests.cs

@@ -0,0 +1,49 @@
+// Copyright (c) Six Labors and contributors.
+// Licensed under the Apache License, Version 2.0.
+
+using System;
+using SixLabors.ImageSharp.MetaData.Profiles.Icc;
+using Xunit;
+
+namespace SixLabors.ImageSharp.Tests.Icc
+{
+    public class IccProfileTests
+    {
+
+#if !NETSTANDARD1_1
+
+        [Theory]
+        [MemberData(nameof(IccTestDataProfiles.ProfileIdTestData), MemberType = typeof(IccTestDataProfiles))]
+        public void CalculateHash_WithByteArray_CalculatesProfileHash(byte[] data, IccProfileId expected)
+        {
+            IccProfileId result = IccProfile.CalculateHash(data);
+
+            Assert.Equal(expected, result);
+        }
+
+        [Fact]
+        public void CalculateHash_WithByteArray_DoesNotModifyData()
+        {
+            byte[] data = IccTestDataProfiles.Profile_Random_Array;
+            byte[] copy = new byte[data.Length];
+            Buffer.BlockCopy(data, 0, copy, 0, data.Length);
+
+            IccProfileId result = IccProfile.CalculateHash(data);
+
+            Assert.Equal(data, copy);
+        }
+
+#endif
+
+        [Theory]
+        [MemberData(nameof(IccTestDataProfiles.ProfileValidityTestData), MemberType = typeof(IccTestDataProfiles))]
+        public void CheckIsValid_WithProfiles_ReturnsValidity(byte[] data, bool expected)
+        {
+            var profile = new IccProfile(data);
+
+            bool result = profile.CheckIsValid();
+
+            Assert.Equal(expected, result);
+        }
+    }
+}

tests/ImageSharp.Tests/TestDataIcc/IccTestDataProfiles.cs

@@ -9,6 +9,27 @@ namespace SixLabors.ImageSharp.Tests
 {
     internal static class IccTestDataProfiles
     {
+        public static readonly IccProfileId Header_Random_Id_Value = new IccProfileId(0x84A8D460, 0xC716B6F3, 0x9B0E4C3D, 0xAB95F838);
+        public static readonly IccProfileId Profile_Random_Id_Value = new IccProfileId(0x917D6DE6, 0x84C958D1, 0x3BB0F5BB, 0xADD1134F);
+
+        public static readonly byte[] Header_Random_Id_Array =
+        {
+#if !NETSTANDARD1_1
+                0x84, 0xA8, 0xD4, 0x60, 0xC7, 0x16, 0xB6, 0xF3, 0x9B, 0x0E, 0x4C, 0x3D, 0xAB, 0x95, 0xF8, 0x38,
+#else
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+#endif
+        };
+
+        public static readonly byte[] Profile_Random_Id_Array =
+        {
+#if !NETSTANDARD1_1
+                0x91, 0x7D, 0x6D, 0xE6, 0x84, 0xC9, 0x58, 0xD1, 0x3B, 0xB0, 0xF5, 0xBB, 0xAD, 0xD1, 0x13, 0x4F,
+#else
+                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+#endif
+        };
+
         public static readonly IccProfileHeader Header_Random_Write = CreateHeaderRandomValue(
             562,        // should be overwritten
             new IccProfileId(1, 2, 3, 4),   // should be overwritten
@@ -16,20 +37,13 @@ namespace SixLabors.ImageSharp.Tests
 
         public static readonly IccProfileHeader Header_Random_Read = CreateHeaderRandomValue(132,
 #if !NETSTANDARD1_1
-            new IccProfileId(2931428592, 418415738, 3086756963, 2237536530),
+            Header_Random_Id_Value,
 #else
             IccProfileId.Zero,
 #endif
             "acsp");
 
-        public static readonly byte[] Header_Random_Array = CreateHeaderRandomArray(132, 0, new byte[]
-        {
-#if !NETSTANDARD1_1
-                0xAE, 0xBA, 0x0C, 0xF0, 0x18, 0xF0, 0x84, 0x7A, 0xB7, 0xFC, 0x2C, 0x63, 0x85, 0x5E, 0x19, 0x12,
-#else
-                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-#endif
-        });
+        public static readonly byte[] Header_Random_Array = CreateHeaderRandomArray(132, 0, Header_Random_Id_Array);
 
         public static IccProfileHeader CreateHeaderRandomValue(uint size, IccProfileId id, string fileSignature)
         {
@@ -45,11 +59,7 @@ namespace SixLabors.ImageSharp.Tests
                 DeviceModel = 987654321u,
                 FileSignature = "acsp",
                 Flags = IccProfileFlag.Embedded | IccProfileFlag.Independent,
-#if !NETSTANDARD1_1
-                Id = new IccProfileId(2931428592, 418415738, 3086756963, 2237536530),
-#else
-            Id = IccProfileId.Zero,
-#endif
+                Id = id,
                 PcsIlluminant = new Vector3(4, 5, 6),
                 PrimaryPlatformSignature = IccPrimaryPlatformType.MicrosoftCorporation,
                 ProfileConnectionSpace = IccColorSpaceType.CieXyz,
@@ -94,14 +104,7 @@ namespace SixLabors.ImageSharp.Tests
                  });
         }
 
-        public static byte[] Profile_Random_Array = ArrayHelper.Concat(CreateHeaderRandomArray(168, 2, new byte[] 
-            {
-#if !NETSTANDARD1_1
-                0xA9, 0x71, 0x8F, 0xC1, 0x1E, 0x2D, 0x64, 0x1B, 0x10, 0xF4, 0x7D, 0x6A, 0x5B, 0xF6, 0xAC, 0xB9
-#else
-                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-#endif
-            }),
+        public static byte[] Profile_Random_Array = ArrayHelper.Concat(CreateHeaderRandomArray(168, 2, Profile_Random_Id_Array),
             new byte[]
             {
                 0x00, 0x00, 0x00, 0x00,     // tag signature (Unknown)
@@ -118,7 +121,7 @@ namespace SixLabors.ImageSharp.Tests
 
         public static IccProfile Profile_Random_Val = new IccProfile(CreateHeaderRandomValue(168,
 #if !NETSTANDARD1_1
-            new IccProfileId(0xA9718FC1, 0x1E2D641B, 0x10F47D6A, 0x5BF6ACB9),
+            Profile_Random_Id_Value,
 #else
             IccProfileId.Zero,
 #endif
@@ -128,5 +131,43 @@ namespace SixLabors.ImageSharp.Tests
                 IccTestDataTagDataEntry.Unknown_Val,
                 IccTestDataTagDataEntry.Unknown_Val
             });
+
+        public static byte[] Header_Corrupt1_Array =
+        {
+            0x81, 0xB1, 0x81, 0xE4, 0x82, 0x16, 0x82, 0x49, 0x82, 0x7B, 0x82, 0xAD, 0x82, 0xDF, 0x83, 0x11,
+            0x83, 0x43, 0x83, 0x75, 0x83, 0xA7, 0x83, 0xD8, 0x84, 0x0A, 0x84, 0x3B, 0x84, 0x6C, 0x84, 0x9E,
+            0x84, 0xCF, 0x85, 0x00, 0x85, 0x31, 0x85, 0x62, 0x85, 0x93, 0x85, 0xC3, 0x85, 0xF4, 0x86, 0x24,
+            0x86, 0x55, 0x86, 0x85, 0x86, 0xB5, 0x86, 0xE6, 0x87, 0x16, 0x87, 0x46, 0x87, 0x76, 0x87, 0xA5,
+            0x87, 0xD5, 0x88, 0x05, 0x88, 0x34, 0x88, 0x64, 0x88, 0x93, 0x88, 0xC3, 0x88, 0xF2, 0x89, 0x21,
+            0x89, 0x50, 0x89, 0x7F, 0x89, 0xAE, 0x89, 0xDD, 0x8A, 0x0C, 0x8A, 0x3B, 0x8A, 0x69, 0x8A, 0x98,
+            0x8A, 0xC6, 0x8A, 0xF5, 0x8B, 0x23, 0x8B, 0x51, 0x8B, 0x7F, 0x8B, 0xAE, 0x8B, 0xDC, 0x8C, 0x09,
+            0x8C, 0x37, 0x8C, 0x65, 0x8C, 0x93, 0x8C, 0xC1, 0x8C, 0xEE, 0x8D, 0x1C, 0x8D, 0x49, 0x8D, 0x76,
+        };
+
+        public static byte[] Header_Corrupt2_Array =
+        {
+            0x23, 0x74, 0x6D, 0x6D, 0xB1, 0xBC, 0x28, 0xB2, 0x6D, 0x0B, 0xA3, 0x9C, 0x2D, 0x60, 0x6C, 0xB4,
+            0x96, 0xF2, 0x31, 0x88, 0x6C, 0x67, 0x8B, 0xA9, 0x35, 0x31, 0x6C, 0x24, 0x81, 0xAE, 0x38, 0x64,
+            0x6B, 0xE9, 0x78, 0xEC, 0x3B, 0x28, 0x6B, 0xB7, 0x71, 0x4F, 0x3D, 0x87, 0x6B, 0x8C, 0x6A, 0xC3,
+            0x3F, 0x87, 0x6B, 0x68, 0x65, 0x33, 0x41, 0x30, 0x6B, 0x4A, 0x60, 0x8C, 0x42, 0x8C, 0x6B, 0x32,
+            0x5C, 0xB8, 0x43, 0xA2, 0x6B, 0x1F, 0x59, 0xA4, 0x44, 0x79, 0x6B, 0x10, 0x57, 0x3B, 0x45, 0x1A,
+            0x6B, 0x05, 0x55, 0x68, 0x45, 0x8D, 0x6A, 0xFE, 0x54, 0x15, 0x45, 0xDA, 0x6A, 0xF9, 0x53, 0x2A,
+            0x46, 0x16, 0x6A, 0xF5, 0x52, 0x74, 0x46, 0x27, 0x6A, 0xF4, 0x52, 0x43, 0x46, 0x27, 0x6A, 0xF4,
+            0x52, 0x43, 0x46, 0x27, 0x6A, 0xF4, 0x52, 0x43, 0x46, 0x27, 0x6A, 0xF4, 0x52, 0x43, 0x46, 0x27,
+        };
+
+
+        public static object[][] ProfileIdTestData =
+        {
+            new object[] { Header_Random_Array, Header_Random_Id_Value },
+            new object[] { Profile_Random_Array, Profile_Random_Id_Value },
+        };
+
+        public static object[][] ProfileValidityTestData =
+        {
+            new object[] { Header_Corrupt1_Array, false },
+            new object[] { Header_Corrupt2_Array, false },
+            new object[] { Header_Random_Array, true },
+        };
     }
 }