Merge pull request #1944 from SixLabors/bp/issue1942

Fix for issue #1942

src/ImageSharp/Metadata/Profiles/IPTC/IptcProfile.cs

@@ -16,6 +16,10 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Iptc
     {
         private Collection<IptcValue> values;
 
+        private const byte IptcTagMarkerByte = 0x1c;
+
+        private const uint MaxStandardDataTagSize = 0x7FFF;
+
         /// <summary>
         /// Initializes a new instance of the <see cref="IptcProfile"/> class.
         /// </summary>
@@ -78,7 +82,7 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Iptc
         }
 
         /// <inheritdoc/>
-        public IptcProfile DeepClone() => new IptcProfile(this);
+        public IptcProfile DeepClone() => new(this);
 
         /// <summary>
         /// Returns all values with the specified tag.
@@ -207,7 +211,7 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Iptc
                 throw new ArgumentException("iptc tag is not a time or date type");
             }
 
-            var formattedDate = tag.IsDate()
+            string formattedDate = tag.IsDate()
                 ? dateTimeOffset.ToString("yyyyMMdd", System.Globalization.CultureInfo.InvariantCulture)
                 : dateTimeOffset.ToString("HHmmsszzzz", System.Globalization.CultureInfo.InvariantCulture)
                     .Replace(":", string.Empty);
@@ -231,7 +235,7 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Iptc
         /// </summary>
         public void UpdateData()
         {
-            var length = 0;
+            int length = 0;
             foreach (IptcValue value in this.Values)
             {
                 length += value.Length + 5;
@@ -242,7 +246,24 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Iptc
             int i = 0;
             foreach (IptcValue value in this.Values)
             {
-                this.Data[i++] = 28;
+                // Standard DataSet Tag
+                // +-----------+----------------+---------------------------------------------------------------------------------+
+                // | Octet Pos | Name           | Description                                                                     |
+                // +==========-+================+=================================================================================+
+                // | 1         | Tag Marker     | Is the tag marker that initiates the start of a DataSet 0x1c.                   |
+                // +-----------+----------------+---------------------------------------------------------------------------------+
+                // | 2         | Record Number  | Octet 2 is the binary representation of the record number. Note that the        |
+                // |           |                | envelope record number is always 1, and that the application records are        |
+                // |           |                | numbered 2 through 6, the pre-object descriptor record is 7, the object record  |
+                // |           |                | is 8, and the post - object descriptor record is 9.                             |
+                // +-----------+----------------+---------------------------------------------------------------------------------+
+                // | 3         | DataSet Number | Octet 3 is the binary representation of the DataSet number.                     |
+                // +-----------+----------------+---------------------------------------------------------------------------------+
+                // | 4 and 5   | Data Field     | Octets 4 and 5, taken together, are the binary count of the number of octets in |
+                // |           | Octet Count    | the following data field(32767 or fewer octets). Note that the value of bit 7 of|
+                // |           |                | octet 4(most significant bit) always will be 0.                                 |
+                // +-----------+----------------+---------------------------------------------------------------------------------+
+                this.Data[i++] = IptcTagMarkerByte;
                 this.Data[i++] = 2;
                 this.Data[i++] = (byte)value.Tag;
                 this.Data[i++] = (byte)(value.Length >> 8);
@@ -264,34 +285,36 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Iptc
 
             this.values = new Collection<IptcValue>();
 
-            if (this.Data == null || this.Data[0] != 0x1c)
+            if (this.Data == null || this.Data[0] != IptcTagMarkerByte)
             {
                 return;
             }
 
-            int i = 0;
+            int offset = 0;
-            while (i + 4 < this.Data.Length)
+            while (offset < this.Data.Length - 4)
             {
-                if (this.Data[i++] != 28)
+                bool isValidTagMarker = this.Data[offset++] == IptcTagMarkerByte;
+                byte recordNumber = this.Data[offset++];
+                bool isValidRecordNumber = recordNumber is >= 1 and <= 9;
+                var tag = (IptcTag)this.Data[offset++];
+                bool isValidEntry = isValidTagMarker && isValidRecordNumber;
+
+                uint byteCount = BinaryPrimitives.ReadUInt16BigEndian(this.Data.AsSpan(offset, 2));
+                offset += 2;
+                if (byteCount > MaxStandardDataTagSize)
                 {
-                    continue;
+                    // Extended data set tag's are not supported.
+                    break;
                 }
 
-                i++;
+                if (isValidEntry && byteCount > 0 && (offset <= this.Data.Length - byteCount))
-
-                var tag = (IptcTag)this.Data[i++];
-
-                int count = BinaryPrimitives.ReadInt16BigEndian(this.Data.AsSpan(i, 2));
-                i += 2;
-
-                var iptcData = new byte[count];
-                if ((count > 0) && (i + count <= this.Data.Length))
                 {
-                    Buffer.BlockCopy(this.Data, i, iptcData, 0, count);
+                    var iptcData = new byte[byteCount];
+                    Buffer.BlockCopy(this.Data, offset, iptcData, 0, (int)byteCount);
                     this.values.Add(new IptcValue(tag, iptcData, false));
                 }
 
-                i += count;
+                offset += (int)byteCount;
             }
         }
     }

src/ImageSharp/Metadata/Profiles/IPTC/IptcTagExtensions.cs

@@ -13,9 +13,7 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Iptc
         /// </summary>
         /// <param name="tag">The tag to check the max length for.</param>
         /// <returns>The maximum length.</returns>
-        public static int MaxLength(this IptcTag tag)
+        public static int MaxLength(this IptcTag tag) => tag switch
-        {
-            return tag switch
         {
             IptcTag.RecordVersion => 2,
             IptcTag.ObjectType => 67,
@@ -66,7 +64,6 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Iptc
             IptcTag.ImageOrientation => 1,
             _ => 256
         };
-        }
 
         /// <summary>
         /// Determines if the given tag can be repeated according to the specification.

src/ImageSharp/Metadata/Profiles/IPTC/IptcValue.cs

@@ -101,7 +101,7 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Iptc
                     byte[] valueBytes;
                     if (this.Strict && value.Length > maxLength)
                     {
-                        var cappedValue = value.Substring(0, maxLength);
+                        string cappedValue = value.Substring(0, maxLength);
                         valueBytes = this.encoding.GetBytes(cappedValue);
 
                         // It is still possible that the bytes of the string exceed the limit.

tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs

@@ -288,5 +288,17 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg
                         Assert.Equal(72, imageInfo.Metadata.HorizontalResolution);
                         Assert.Equal(72, imageInfo.Metadata.VerticalResolution);
                     });
+
+        [Theory]
+        [WithFile(TestImages.Jpeg.Issues.InvalidIptcTag, PixelTypes.Rgba32)]
+        public void Decode_WithInvalidIptcTag_DoesNotThrowException<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : unmanaged, IPixel<TPixel>
+        {
+            Exception ex = Record.Exception(() =>
+            {
+                using Image<TPixel> image = provider.GetImage(JpegDecoder);
+            });
+            Assert.Null(ex);
+        }
     }
 }

tests/ImageSharp.Tests/TestImages.cs

@@ -261,6 +261,7 @@ namespace SixLabors.ImageSharp.Tests
                 public const string WrongColorSpace = "Jpg/issues/Issue1732-WrongColorSpace.jpg";
                 public const string MalformedUnsupportedComponentCount = "Jpg/issues/issue-1900-malformed-unsupported-255-components.jpg";
                 public const string MultipleApp01932 = "Jpg/issues/issue-1932-app0-resolution.jpg";
+                public const string InvalidIptcTag = "Jpg/issues/Issue1942InvalidIptcTag.jpg";
 
                 public static class Fuzz
                 {

tests/Images/Input/Jpg/issues/Issue1942InvalidIptcTag.jpg

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:9c9db428c4d9d7d1aea6778f263d8deaeeabdcfa63c77ef6ce36ab0e47b364dd
+size 93374