From d3936e2d3ff5342df8723244bc89479048a598e3 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 27 Mar 2022 22:47:44 +1100 Subject: [PATCH 01/13] Add check for App1 XMP marker length --- src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index ef4e3ffac2..51e0675754 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -722,7 +722,14 @@ private void ProcessApp1Marker(BufferedReadStream stream, int remaining) if (ProfileResolver.IsProfile(this.temp, ProfileResolver.XmpMarker.Slice(0, ExifMarkerLength))) { - int remainingXmpMarkerBytes = XmpMarkerLength - ExifMarkerLength; + const int remainingXmpMarkerBytes = XmpMarkerLength - ExifMarkerLength; + if (remaining < remainingXmpMarkerBytes || this.IgnoreMetadata) + { + // Skip the application header length. + stream.Skip(remaining); + return; + } + stream.Read(this.temp, ExifMarkerLength, remainingXmpMarkerBytes); remaining -= remainingXmpMarkerBytes; if (ProfileResolver.IsProfile(this.temp, ProfileResolver.XmpMarker)) From 96f6fd45a316e33b78026c8c40868928643775ef Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sun, 27 Mar 2022 14:17:32 +0200 Subject: [PATCH 02/13] Add check, if enough data is read in LoadTables --- .../Formats/Jpeg/JpegDecoderCore.cs | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 51e0675754..41effc865d 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -232,7 +232,12 @@ public void LoadTables(byte[] tableBytes, HuffmanScanDecoder huffmanScanDecoder) using var stream = new BufferedReadStream(this.Configuration, ms); // Check for the Start Of Image marker. - stream.Read(this.markerBuffer, 0, 2); + int bytesRead = stream.Read(this.markerBuffer, 0, 2); + if (bytesRead != 2) + { + JpegThrowHelper.ThrowInvalidImageContentException("Not enough data to read the SOI marker"); + } + var fileMarker = new JpegFileMarker(this.markerBuffer[1], 0); if (fileMarker.Marker != JpegConstants.Markers.SOI) { @@ -240,7 +245,12 @@ public void LoadTables(byte[] tableBytes, HuffmanScanDecoder huffmanScanDecoder) } // Read next marker. - stream.Read(this.markerBuffer, 0, 2); + bytesRead = stream.Read(this.markerBuffer, 0, 2); + if (bytesRead != 2) + { + JpegThrowHelper.ThrowInvalidImageContentException("Not enough data to read marker"); + } + byte marker = this.markerBuffer[1]; fileMarker = new JpegFileMarker(marker, (int)stream.Position - 2); @@ -273,7 +283,12 @@ public void LoadTables(byte[] tableBytes, HuffmanScanDecoder huffmanScanDecoder) } // Read next marker. - stream.Read(this.markerBuffer, 0, 2); + bytesRead = stream.Read(this.markerBuffer, 0, 2); + if (bytesRead != 2) + { + JpegThrowHelper.ThrowInvalidImageContentException("Not enough data to read marker"); + } + fileMarker = new JpegFileMarker(this.markerBuffer[1], 0); } } From 588a6d444a2aec33e4d93b65d2def63cd8a7ba2c Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sun, 27 Mar 2022 15:18:59 +0200 Subject: [PATCH 03/13] Fix lossy image with not enough exif bytes --- tests/Images/Input/Webp/exif_lossy.webp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Images/Input/Webp/exif_lossy.webp b/tests/Images/Input/Webp/exif_lossy.webp index 35e454b96f..5d6db3800f 100644 --- a/tests/Images/Input/Webp/exif_lossy.webp +++ b/tests/Images/Input/Webp/exif_lossy.webp @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:fdf4e9b20af4168f4177d33f7f502906343bbaaae2af9b90e1531bd4452b317b -size 40765 +oid sha256:5c53967bfefcfece8cd4411740c1c394e75864ca61a7a9751df3b28e727c0205 +size 68646 From f820b0b965716de398974a23a02ac95616580522 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sun, 27 Mar 2022 15:21:30 +0200 Subject: [PATCH 04/13] ParseOptional chunks now checks, if enough data is read --- .../Formats/Webp/WebpDecoderCore.cs | 29 ++++++++++++++++--- .../Formats/Webp/WebpThrowHelper.cs | 7 +++++ 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs index 9d18e5d821..412ec2760a 100644 --- a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs +++ b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs @@ -426,6 +426,7 @@ private WebpImageInfo ReadVp8LHeader(WebpFeatures features = null) /// The webp image features. private void ParseOptionalExtendedChunks(WebpChunkType chunkType, WebpFeatures features) { + int bytesRead; switch (chunkType) { case WebpChunkType.Iccp: @@ -437,7 +438,12 @@ private void ParseOptionalExtendedChunks(WebpChunkType chunkType, WebpFeatures f else { byte[] iccpData = new byte[iccpChunkSize]; - this.currentStream.Read(iccpData, 0, (int)iccpChunkSize); + bytesRead = this.currentStream.Read(iccpData, 0, (int)iccpChunkSize); + if (bytesRead != iccpChunkSize) + { + WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the iccp chunk"); + } + var profile = new IccProfile(iccpData); if (profile.CheckIsValid()) { @@ -456,7 +462,12 @@ private void ParseOptionalExtendedChunks(WebpChunkType chunkType, WebpFeatures f else { byte[] exifData = new byte[exifChunkSize]; - this.currentStream.Read(exifData, 0, (int)exifChunkSize); + bytesRead = this.currentStream.Read(exifData, 0, (int)exifChunkSize); + if (bytesRead != exifChunkSize) + { + WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the exif chunk"); + } + var profile = new ExifProfile(exifData); this.Metadata.ExifProfile = profile; } @@ -472,7 +483,12 @@ private void ParseOptionalExtendedChunks(WebpChunkType chunkType, WebpFeatures f else { byte[] xmpData = new byte[xmpChunkSize]; - this.currentStream.Read(xmpData, 0, (int)xmpChunkSize); + bytesRead = this.currentStream.Read(xmpData, 0, (int)xmpChunkSize); + if (bytesRead != xmpChunkSize) + { + WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the xmp chunk"); + } + var profile = new XmpProfile(xmpData); this.Metadata.XmpProfile = profile; } @@ -488,7 +504,12 @@ private void ParseOptionalExtendedChunks(WebpChunkType chunkType, WebpFeatures f features.AlphaChunkHeader = (byte)this.currentStream.ReadByte(); int alphaDataSize = (int)(alphaChunkSize - 1); features.AlphaData = this.memoryAllocator.Allocate(alphaDataSize); - this.currentStream.Read(features.AlphaData.Memory.Span, 0, alphaDataSize); + bytesRead = this.currentStream.Read(features.AlphaData.Memory.Span, 0, alphaDataSize); + if (bytesRead != alphaDataSize) + { + WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the alpha chunk"); + } + break; default: WebpThrowHelper.ThrowImageFormatException("Unexpected chunk followed VP8X header"); diff --git a/src/ImageSharp/Formats/Webp/WebpThrowHelper.cs b/src/ImageSharp/Formats/Webp/WebpThrowHelper.cs index fffdd34101..5f063fd11a 100644 --- a/src/ImageSharp/Formats/Webp/WebpThrowHelper.cs +++ b/src/ImageSharp/Formats/Webp/WebpThrowHelper.cs @@ -8,6 +8,13 @@ namespace SixLabors.ImageSharp.Formats.Webp { internal static class WebpThrowHelper { + /// + /// Cold path optimization for throwing 's. + /// + /// The error message for the exception. + [MethodImpl(InliningOptions.ColdPath)] + public static void ThrowInvalidImageContentException(string errorMessage) => throw new InvalidImageContentException(errorMessage); + /// /// Cold path optimization for throwing -s /// From 853b222af50851a9d46f73df0042ecc88b467bba Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sun, 27 Mar 2022 15:30:15 +0200 Subject: [PATCH 05/13] Add test for lossy with exif which does not contain enough data --- .../Formats/WebP/WebpMetaDataTests.cs | 12 ++++++++++++ tests/ImageSharp.Tests/TestImages.cs | 1 + .../Input/Webp/exif_lossy_not_enough_data.webp | 3 +++ 3 files changed, 16 insertions(+) create mode 100644 tests/Images/Input/Webp/exif_lossy_not_enough_data.webp diff --git a/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs b/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs index eaa7fb5646..499b0579b6 100644 --- a/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs +++ b/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs @@ -150,5 +150,17 @@ public void EncodeLosslessWebp_PreservesExif(TestImageProvider p Assert.NotNull(actualExif); Assert.Equal(expectedExif.Values.Count, actualExif.Values.Count); } + + [Theory] + [WithFile(TestImages.Webp.Lossy.WithExifNotEnoughData, PixelTypes.Rgba32)] + public void WebpDecoder_ThrowInvalidImageContentException_OnWithInvalidExifData(TestImageProvider provider) + where TPixel : unmanaged, IPixel => + Assert.Throws( + () => + { + using (provider.GetImage(WebpDecoder)) + { + } + }); } } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 07aff6fc12..ac060fd99f 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -635,6 +635,7 @@ public static class Lossy { public const string Earth = "Webp/earth_lossy.webp"; public const string WithExif = "Webp/exif_lossy.webp"; + public const string WithExifNotEnoughData = "Webp/exif_lossy_not_enough_data.webp"; public const string WithIccp = "Webp/lossy_with_iccp.webp"; public const string WithXmp = "Webp/xmp_lossy.webp"; public const string BikeSmall = "Webp/bike_lossless_small.webp"; diff --git a/tests/Images/Input/Webp/exif_lossy_not_enough_data.webp b/tests/Images/Input/Webp/exif_lossy_not_enough_data.webp new file mode 100644 index 0000000000..35e454b96f --- /dev/null +++ b/tests/Images/Input/Webp/exif_lossy_not_enough_data.webp @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:fdf4e9b20af4168f4177d33f7f502906343bbaaae2af9b90e1531bd4452b317b +size 40765 From 5d04734c324a0930ff43f90b9f236a5e8d39e247 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sun, 27 Mar 2022 18:59:04 +0200 Subject: [PATCH 06/13] Add checks in ReadVp8Header() to make sure enough data is read --- .../Formats/Webp/WebpDecoderCore.cs | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs index 412ec2760a..9d83591c36 100644 --- a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs +++ b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs @@ -272,7 +272,12 @@ private WebpImageInfo ReadVp8Header(WebpFeatures features = null) this.webpMetadata.FileFormat = WebpFileFormatType.Lossy; // VP8 data size (not including this 4 bytes). - this.currentStream.Read(this.buffer, 0, 4); + int bytesRead = this.currentStream.Read(this.buffer, 0, 4); + if (bytesRead != 4) + { + WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the VP8 data size"); + } + uint dataSize = BinaryPrimitives.ReadUInt32LittleEndian(this.buffer); // remaining counts the available image data payload. @@ -284,7 +289,12 @@ private WebpImageInfo ReadVp8Header(WebpFeatures features = null) // - A 3-bit version number. // - A 1-bit show_frame flag. // - A 19-bit field containing the size of the first data partition in bytes. - this.currentStream.Read(this.buffer, 0, 3); + bytesRead = this.currentStream.Read(this.buffer, 0, 3); + if (bytesRead != 3) + { + WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the VP8 frame tag"); + } + uint frameTag = (uint)(this.buffer[0] | (this.buffer[1] << 8) | (this.buffer[2] << 16)); remaining -= 3; bool isNoKeyFrame = (frameTag & 0x1) == 1; @@ -312,13 +322,23 @@ private WebpImageInfo ReadVp8Header(WebpFeatures features = null) } // Check for VP8 magic bytes. - this.currentStream.Read(this.buffer, 0, 3); + bytesRead = this.currentStream.Read(this.buffer, 0, 3); + if (bytesRead != 3) + { + WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the VP8 magic bytes"); + } + if (!this.buffer.AsSpan(0, 3).SequenceEqual(WebpConstants.Vp8HeaderMagicBytes)) { WebpThrowHelper.ThrowImageFormatException("VP8 magic bytes not found"); } - this.currentStream.Read(this.buffer, 0, 4); + bytesRead = this.currentStream.Read(this.buffer, 0, 4); + if (bytesRead != 4) + { + WebpThrowHelper.ThrowInvalidImageContentException("VP8 header does not contain enough data to read the image width and height"); + } + uint tmp = (uint)BinaryPrimitives.ReadInt16LittleEndian(this.buffer); uint width = tmp & 0x3fff; sbyte xScale = (sbyte)(tmp >> 6); From 8cc5a6b6cad3bccd6b7a04855428b8f78ce0a517 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sun, 27 Mar 2022 19:14:42 +0200 Subject: [PATCH 07/13] Remove code duplication when reading the profile data --- .../Formats/Webp/WebpDecoderCore.cs | 145 ++++++++++-------- 1 file changed, 85 insertions(+), 60 deletions(-) diff --git a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs index 9d83591c36..2a840ba709 100644 --- a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs +++ b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs @@ -450,68 +450,17 @@ private void ParseOptionalExtendedChunks(WebpChunkType chunkType, WebpFeatures f switch (chunkType) { case WebpChunkType.Iccp: - uint iccpChunkSize = this.ReadChunkSize(); - if (this.IgnoreMetadata) - { - this.currentStream.Skip((int)iccpChunkSize); - } - else - { - byte[] iccpData = new byte[iccpChunkSize]; - bytesRead = this.currentStream.Read(iccpData, 0, (int)iccpChunkSize); - if (bytesRead != iccpChunkSize) - { - WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the iccp chunk"); - } - - var profile = new IccProfile(iccpData); - if (profile.CheckIsValid()) - { - this.Metadata.IccProfile = profile; - } - } + this.ReadIccProfile(); break; case WebpChunkType.Exif: - uint exifChunkSize = this.ReadChunkSize(); - if (this.IgnoreMetadata) - { - this.currentStream.Skip((int)exifChunkSize); - } - else - { - byte[] exifData = new byte[exifChunkSize]; - bytesRead = this.currentStream.Read(exifData, 0, (int)exifChunkSize); - if (bytesRead != exifChunkSize) - { - WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the exif chunk"); - } - - var profile = new ExifProfile(exifData); - this.Metadata.ExifProfile = profile; - } + this.ReadExifProfile(); break; case WebpChunkType.Xmp: - uint xmpChunkSize = this.ReadChunkSize(); - if (this.IgnoreMetadata) - { - this.currentStream.Skip((int)xmpChunkSize); - } - else - { - byte[] xmpData = new byte[xmpChunkSize]; - bytesRead = this.currentStream.Read(xmpData, 0, (int)xmpChunkSize); - if (bytesRead != xmpChunkSize) - { - WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the xmp chunk"); - } - - var profile = new XmpProfile(xmpData); - this.Metadata.XmpProfile = profile; - } + this.ReadXmpProfile(); break; @@ -555,22 +504,98 @@ private void ParseOptionalChunks(WebpFeatures features) { // Read chunk header. WebpChunkType chunkType = this.ReadChunkType(); - uint chunkLength = this.ReadChunkSize(); - if (chunkType == WebpChunkType.Exif && this.Metadata.ExifProfile == null) { - byte[] exifData = new byte[chunkLength]; - this.currentStream.Read(exifData, 0, (int)chunkLength); - this.Metadata.ExifProfile = new ExifProfile(exifData); + this.ReadExifProfile(); + } + else if (chunkType == WebpChunkType.Xmp && this.Metadata.XmpProfile == null) + { + this.ReadXmpProfile(); } else { - // Skip XMP chunk data or any duplicate EXIF chunk. + // Skip duplicate XMP or EXIF chunk. + uint chunkLength = this.ReadChunkSize(); this.currentStream.Skip((int)chunkLength); } } } + /// + /// Reads the EXIF profile from the stream. + /// + private void ReadExifProfile() + { + uint exifChunkSize = this.ReadChunkSize(); + if (this.IgnoreMetadata) + { + this.currentStream.Skip((int)exifChunkSize); + } + else + { + byte[] exifData = new byte[exifChunkSize]; + int bytesRead = this.currentStream.Read(exifData, 0, (int)exifChunkSize); + if (bytesRead != exifChunkSize) + { + WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the exif chunk"); + } + + var profile = new ExifProfile(exifData); + this.Metadata.ExifProfile = profile; + } + } + + /// + /// Reads the XMP profile the stream. + /// + private void ReadXmpProfile() + { + uint xmpChunkSize = this.ReadChunkSize(); + if (this.IgnoreMetadata) + { + this.currentStream.Skip((int)xmpChunkSize); + } + else + { + byte[] xmpData = new byte[xmpChunkSize]; + int bytesRead = this.currentStream.Read(xmpData, 0, (int)xmpChunkSize); + if (bytesRead != xmpChunkSize) + { + WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the xmp chunk"); + } + + var profile = new XmpProfile(xmpData); + this.Metadata.XmpProfile = profile; + } + } + + /// + /// Reads the ICCP chunk from the stream. + /// + private void ReadIccProfile() + { + uint iccpChunkSize = this.ReadChunkSize(); + if (this.IgnoreMetadata) + { + this.currentStream.Skip((int)iccpChunkSize); + } + else + { + byte[] iccpData = new byte[iccpChunkSize]; + int bytesRead = this.currentStream.Read(iccpData, 0, (int)iccpChunkSize); + if (bytesRead != iccpChunkSize) + { + WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the iccp chunk"); + } + + var profile = new IccProfile(iccpData); + if (profile.CheckIsValid()) + { + this.Metadata.IccProfile = profile; + } + } + } + /// /// Identifies the chunk type from the chunk. /// From 8e4129a7335ebb2a1a1f977654eeb7db6389cd1c Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sun, 27 Mar 2022 19:20:14 +0200 Subject: [PATCH 08/13] Make sure enough data is read in ReadVp8XHeader() --- .../Formats/Webp/WebpDecoderCore.cs | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs index 2a840ba709..91594a6740 100644 --- a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs +++ b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs @@ -190,7 +190,11 @@ private WebpImageInfo ReadVp8XHeader() uint fileSize = this.ReadChunkSize(); // The first byte contains information about the image features used. - byte imageFeatures = (byte)this.currentStream.ReadByte(); + int imageFeatures = this.currentStream.ReadByte(); + if (imageFeatures == -1) + { + WebpThrowHelper.ThrowInvalidImageContentException("VP8X header doe not contain enough data"); + } // The first two bit of it are reserved and should be 0. if (imageFeatures >> 6 != 0) @@ -214,19 +218,34 @@ private WebpImageInfo ReadVp8XHeader() features.Animation = (imageFeatures & (1 << 1)) != 0; // 3 reserved bytes should follow which are supposed to be zero. - this.currentStream.Read(this.buffer, 0, 3); + int bytesRead = this.currentStream.Read(this.buffer, 0, 3); + if (bytesRead != 3) + { + WebpThrowHelper.ThrowInvalidImageContentException("VP8X header does not contain enough data"); + } + if (this.buffer[0] != 0 || this.buffer[1] != 0 || this.buffer[2] != 0) { WebpThrowHelper.ThrowImageFormatException("reserved bytes should be zero"); } // 3 bytes for the width. - this.currentStream.Read(this.buffer, 0, 3); + bytesRead = this.currentStream.Read(this.buffer, 0, 3); + if (bytesRead != 3) + { + WebpThrowHelper.ThrowInvalidImageContentException("VP8 header does not contain enough data to read the width"); + } + this.buffer[3] = 0; uint width = (uint)BinaryPrimitives.ReadInt32LittleEndian(this.buffer) + 1; // 3 bytes for the height. - this.currentStream.Read(this.buffer, 0, 3); + bytesRead = this.currentStream.Read(this.buffer, 0, 3); + if (bytesRead != 3) + { + WebpThrowHelper.ThrowInvalidImageContentException("VP8 header does not contain enough data to read the height"); + } + this.buffer[3] = 0; uint height = (uint)BinaryPrimitives.ReadInt32LittleEndian(this.buffer) + 1; From aac680fea60968320220c09492d8aa6216910295 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sun, 27 Mar 2022 19:28:03 +0200 Subject: [PATCH 09/13] Add check, if enough data was read for progressive scan decoding data --- src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 41effc865d..56bdca5f8d 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -1334,8 +1334,12 @@ private void ProcessStartOfScanMarker(BufferedReadStream stream, int remaining) component.ACHuffmanTableId = acTableIndex; } - // 3 bytes: Progressive scan decoding data - stream.Read(this.temp, 0, 3); + // 3 bytes: Progressive scan decoding data. + int bytesRead = stream.Read(this.temp, 0, 3); + if (bytesRead != 3) + { + JpegThrowHelper.ThrowInvalidImageContentException("Not enough data to read progressive scan decoding data"); + } int spectralStart = this.temp[0]; this.scanDecoder.SpectralStart = spectralStart; From e6ad467503ed31af90d7a55920a021e0c8c723ef Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sun, 27 Mar 2022 19:29:12 +0200 Subject: [PATCH 10/13] Add check in ReadUint16 for enough data --- src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 56bdca5f8d..e2d6cfcd29 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -1362,7 +1362,12 @@ private void ProcessStartOfScanMarker(BufferedReadStream stream, int remaining) [MethodImpl(InliningOptions.ShortMethod)] private ushort ReadUint16(BufferedReadStream stream) { - stream.Read(this.markerBuffer, 0, 2); + int bytesRead = stream.Read(this.markerBuffer, 0, 2); + if (bytesRead != 2) + { + JpegThrowHelper.ThrowInvalidImageContentException("jpeg stream does not contain enough data, could not read ushort."); + } + return BinaryPrimitives.ReadUInt16BigEndian(this.markerBuffer); } } From d22e50c0d0b01b75ca02396d745617f7ad81d256 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Mon, 28 Mar 2022 12:22:58 +0200 Subject: [PATCH 11/13] Ignore invalid EXIF or XMP chunks --- .../Formats/Webp/WebpDecoderCore.cs | 12 +++++------- .../Formats/WebP/WebpMetaDataTests.cs | 19 ++++++++++--------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs index 91594a6740..9e709236c3 100644 --- a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs +++ b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs @@ -465,22 +465,18 @@ private WebpImageInfo ReadVp8LHeader(WebpFeatures features = null) /// The webp image features. private void ParseOptionalExtendedChunks(WebpChunkType chunkType, WebpFeatures features) { - int bytesRead; switch (chunkType) { case WebpChunkType.Iccp: this.ReadIccProfile(); - break; case WebpChunkType.Exif: this.ReadExifProfile(); - break; case WebpChunkType.Xmp: this.ReadXmpProfile(); - break; case WebpChunkType.Animation: @@ -492,7 +488,7 @@ private void ParseOptionalExtendedChunks(WebpChunkType chunkType, WebpFeatures f features.AlphaChunkHeader = (byte)this.currentStream.ReadByte(); int alphaDataSize = (int)(alphaChunkSize - 1); features.AlphaData = this.memoryAllocator.Allocate(alphaDataSize); - bytesRead = this.currentStream.Read(features.AlphaData.Memory.Span, 0, alphaDataSize); + int bytesRead = this.currentStream.Read(features.AlphaData.Memory.Span, 0, alphaDataSize); if (bytesRead != alphaDataSize) { WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the alpha chunk"); @@ -556,7 +552,8 @@ private void ReadExifProfile() int bytesRead = this.currentStream.Read(exifData, 0, (int)exifChunkSize); if (bytesRead != exifChunkSize) { - WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the exif chunk"); + // Ignore invalid chunk. + return; } var profile = new ExifProfile(exifData); @@ -580,7 +577,8 @@ private void ReadXmpProfile() int bytesRead = this.currentStream.Read(xmpData, 0, (int)xmpChunkSize); if (bytesRead != xmpChunkSize) { - WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the xmp chunk"); + // Ignore invalid chunk. + return; } var profile = new XmpProfile(xmpData); diff --git a/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs b/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs index 499b0579b6..456b9a3f52 100644 --- a/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs +++ b/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Apache License, Version 2.0. +using System; using System.IO; using System.Threading.Tasks; using SixLabors.ImageSharp.Formats.Webp; @@ -153,14 +154,14 @@ public void EncodeLosslessWebp_PreservesExif(TestImageProvider p [Theory] [WithFile(TestImages.Webp.Lossy.WithExifNotEnoughData, PixelTypes.Rgba32)] - public void WebpDecoder_ThrowInvalidImageContentException_OnWithInvalidExifData(TestImageProvider provider) - where TPixel : unmanaged, IPixel => - Assert.Throws( - () => - { - using (provider.GetImage(WebpDecoder)) - { - } - }); + public void WebpDecoder_IgnoresInvalidExifChunk(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + Exception ex = Record.Exception(() => + { + using Image image = provider.GetImage(); + }); + Assert.Null(ex); + } } } From ced98879ddb048860732aafb1f0678a51e9df1d5 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Mon, 11 Apr 2022 14:49:41 +0200 Subject: [PATCH 12/13] Add checks, if enough data was read --- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index 16dca3324f..d17e89cd45 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -221,7 +221,11 @@ public IImageInfo Identify(BufferedReadStream stream, CancellationToken cancella /// private void ReadGraphicalControlExtension() { - this.stream.Read(this.buffer, 0, 6); + int bytesRead = this.stream.Read(this.buffer, 0, 6); + if (bytesRead != 6) + { + GifThrowHelper.ThrowInvalidImageContentException("Not enough data to read the graphic control extension"); + } this.graphicsControlExtension = GifGraphicControlExtension.Parse(this.buffer); } @@ -231,7 +235,11 @@ private void ReadGraphicalControlExtension() /// private void ReadImageDescriptor() { - this.stream.Read(this.buffer, 0, 9); + int bytesRead = this.stream.Read(this.buffer, 0, 9); + if (bytesRead != 9) + { + GifThrowHelper.ThrowInvalidImageContentException("Not enough data to read the image descriptor"); + } this.imageDescriptor = GifImageDescriptor.Parse(this.buffer); if (this.imageDescriptor.Height == 0 || this.imageDescriptor.Width == 0) @@ -245,7 +253,11 @@ private void ReadImageDescriptor() /// private void ReadLogicalScreenDescriptor() { - this.stream.Read(this.buffer, 0, 7); + int bytesRead = this.stream.Read(this.buffer, 0, 7); + if (bytesRead != 7) + { + GifThrowHelper.ThrowInvalidImageContentException("Not enough data to read the logical screen descriptor"); + } this.logicalScreenDescriptor = GifLogicalScreenDescriptor.Parse(this.buffer); } From 2ac18d816f0074f51a19983662594090516a0a7a Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 16 Apr 2022 18:39:49 +1000 Subject: [PATCH 13/13] Optimize tiff/jpeg checks --- .../Formats/Jpeg/JpegDecoderCore.cs | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index a7f93b0e85..533ffa719c 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -228,16 +228,17 @@ public void LoadTables(byte[] tableBytes, HuffmanScanDecoder huffmanScanDecoder) this.Metadata = new ImageMetadata(); this.QuantizationTables = new Block8x8F[4]; this.scanDecoder = huffmanScanDecoder; + + if (tableBytes.Length < 4) + { + JpegThrowHelper.ThrowInvalidImageContentException("Not enough data to read marker"); + } + using var ms = new MemoryStream(tableBytes); using var stream = new BufferedReadStream(this.Configuration, ms); // Check for the Start Of Image marker. int bytesRead = stream.Read(this.markerBuffer, 0, 2); - if (bytesRead != 2) - { - JpegThrowHelper.ThrowInvalidImageContentException("Not enough data to read the SOI marker"); - } - var fileMarker = new JpegFileMarker(this.markerBuffer[1], 0); if (fileMarker.Marker != JpegConstants.Markers.SOI) { @@ -246,20 +247,22 @@ public void LoadTables(byte[] tableBytes, HuffmanScanDecoder huffmanScanDecoder) // Read next marker. bytesRead = stream.Read(this.markerBuffer, 0, 2); - if (bytesRead != 2) - { - JpegThrowHelper.ThrowInvalidImageContentException("Not enough data to read marker"); - } - - byte marker = this.markerBuffer[1]; - fileMarker = new JpegFileMarker(marker, (int)stream.Position - 2); + fileMarker = new JpegFileMarker(this.markerBuffer[1], (int)stream.Position - 2); while (fileMarker.Marker != JpegConstants.Markers.EOI || (fileMarker.Marker == JpegConstants.Markers.EOI && fileMarker.Invalid)) { if (!fileMarker.Invalid) { // Get the marker length. - int remaining = this.ReadUint16(stream) - 2; + int markerContentByteSize = this.ReadUint16(stream) - 2; + + // Check whether stream actually has enought bytes to read + // markerContentByteSize is always positive so we cast + // to uint to avoid sign extension + if (stream.RemainingBytes < (uint)markerContentByteSize) + { + JpegThrowHelper.ThrowNotEnoughBytesForMarker(fileMarker.Marker); + } switch (fileMarker.Marker) { @@ -269,13 +272,13 @@ public void LoadTables(byte[] tableBytes, HuffmanScanDecoder huffmanScanDecoder) case JpegConstants.Markers.RST7: break; case JpegConstants.Markers.DHT: - this.ProcessDefineHuffmanTablesMarker(stream, remaining); + this.ProcessDefineHuffmanTablesMarker(stream, markerContentByteSize); break; case JpegConstants.Markers.DQT: - this.ProcessDefineQuantizationTablesMarker(stream, remaining); + this.ProcessDefineQuantizationTablesMarker(stream, markerContentByteSize); break; case JpegConstants.Markers.DRI: - this.ProcessDefineRestartIntervalMarker(stream, remaining); + this.ProcessDefineRestartIntervalMarker(stream, markerContentByteSize); break; case JpegConstants.Markers.EOI: return;