diff --git a/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs b/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs index 230526fd4a..519b0d536d 100644 --- a/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs +++ b/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs @@ -1431,12 +1431,8 @@ private void ReadInfoHeader(BufferedReadStream stream) this.infoHeader = BmpInfoHeader.ParseV5(buffer); if (this.infoHeader.ProfileData != 0 && this.infoHeader.ProfileSize != 0) { - // Read color profile. long streamPosition = stream.Position; - byte[] iccProfileData = new byte[this.infoHeader.ProfileSize]; - stream.Position = infoHeaderStart + this.infoHeader.ProfileData; - stream.Read(iccProfileData); - this.metadata.IccProfile = new IccProfile(iccProfileData); + this.ExecuteAncillarySegmentAction(() => this.ReadIccProfile(stream, this.metadata, infoHeaderStart)); stream.Position = streamPosition; } } @@ -1470,6 +1466,33 @@ private void ReadInfoHeader(BufferedReadStream stream) this.Dimensions = new Size(this.infoHeader.Width, this.infoHeader.Height); } + /// + /// Reads the embedded ICC profile from the BMP V5 info header. + /// + /// The containing image data. + /// The image metadata. + /// The stream position where the info header begins. + private void ReadIccProfile(BufferedReadStream stream, ImageMetadata imageMetadata, long infoHeaderStart) + { + byte[] iccProfileData = new byte[this.infoHeader.ProfileSize]; + stream.Position = infoHeaderStart + this.infoHeader.ProfileData; + + if (stream.Read(iccProfileData) != iccProfileData.Length) + { + BmpThrowHelper.ThrowInvalidImageContentException("Not enough data to read BMP ICC profile."); + } + + IccProfile profile = new(iccProfileData); + if (profile.CheckIsValid()) + { + imageMetadata.IccProfile = profile; + } + else + { + throw new InvalidIccProfileException("Invalid BMP ICC profile."); + } + } + /// /// Reads the from the stream. /// diff --git a/src/ImageSharp/Formats/DecoderOptions.cs b/src/ImageSharp/Formats/DecoderOptions.cs index d4d80fc123..916888af33 100644 --- a/src/ImageSharp/Formats/DecoderOptions.cs +++ b/src/ImageSharp/Formats/DecoderOptions.cs @@ -60,7 +60,7 @@ public sealed class DecoderOptions /// /// Gets the segment error handling strategy to use during decoding. /// - public SegmentIntegrityHandling SegmentIntegrityHandling { get; init; } = SegmentIntegrityHandling.IgnoreNonCritical; + public SegmentIntegrityHandling SegmentIntegrityHandling { get; init; } = SegmentIntegrityHandling.IgnoreAncillary; /// /// Gets a value that controls how ICC profiles are handled during decode. diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index 3d32c7cdac..c087b66811 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -146,10 +146,10 @@ protected override Image Decode(BufferedReadStream stream, Cance this.ReadGraphicalControlExtension(stream); break; case GifConstants.CommentLabel: - this.ReadComments(stream); + this.ExecuteAncillarySegmentAction(() => this.ReadComments(stream)); break; case GifConstants.ApplicationExtensionLabel: - this.ReadApplicationExtension(stream); + this.ExecuteAncillarySegmentAction(() => this.ReadApplicationExtension(stream)); break; case GifConstants.PlainTextLabel: SkipBlock(stream); // Not supported by any known decoder. @@ -226,10 +226,10 @@ protected override ImageInfo Identify(BufferedReadStream stream, CancellationTok this.ReadGraphicalControlExtension(stream); break; case GifConstants.CommentLabel: - this.ReadComments(stream); + this.ExecuteAncillarySegmentAction(() => this.ReadComments(stream)); break; case GifConstants.ApplicationExtensionLabel: - this.ReadApplicationExtension(stream); + this.ExecuteAncillarySegmentAction(() => this.ReadApplicationExtension(stream)); break; case GifConstants.PlainTextLabel: SkipBlock(stream); // Not supported by any known decoder. @@ -266,6 +266,13 @@ protected override ImageInfo Identify(BufferedReadStream stream, CancellationTok GifThrowHelper.ThrowNoHeader(); } + // Ignoring a malformed ancillary extension must not let identify succeed for a file + // that never contained any readable image frame data. + if (previousFrame is null) + { + GifThrowHelper.ThrowNoData(); + } + return new ImageInfo( new Size(this.logicalScreenDescriptor.Width, this.logicalScreenDescriptor.Height), this.metadata, @@ -331,51 +338,128 @@ private void ReadLogicalScreenDescriptor(BufferedReadStream stream) private void ReadApplicationExtension(BufferedReadStream stream) { int appLength = stream.ReadByte(); + if (appLength == -1) + { + GifThrowHelper.ThrowInvalidImageContentException("Unexpected end of stream while reading gif application extension"); + } + + if (appLength != GifConstants.ApplicationBlockSize) + { + this.ThrowOrIgnoreNonStrictSegmentError($"Gif application extension length '{appLength}' is invalid"); + SkipBlock(stream, appLength); + return; + } // If the length is 11 then it's a valid extension and most likely // a NETSCAPE, XMP or ANIMEXTS extension. We want the loop count from this. long position = stream.Position; - if (appLength == GifConstants.ApplicationBlockSize) + int bytesRead = stream.Read(this.buffer.Span, 0, GifConstants.ApplicationBlockSize); + if (bytesRead != GifConstants.ApplicationBlockSize) { - stream.Read(this.buffer.Span, 0, GifConstants.ApplicationBlockSize); - bool isXmp = this.buffer.Span.StartsWith(GifConstants.XmpApplicationIdentificationBytes); - if (isXmp && !this.skipMetadata) - { - GifXmpApplicationExtension extension = GifXmpApplicationExtension.Read(stream, this.memoryAllocator); - if (extension.Data.Length > 0) - { - this.metadata!.XmpProfile = new XmpProfile(extension.Data); - } - else - { - // Reset the stream position and continue. - stream.Position = position; - SkipBlock(stream, appLength); - } + GifThrowHelper.ThrowInvalidImageContentException("Unexpected end of stream while reading gif application extension"); + } - return; - } + bool isXmp = this.buffer.Span.StartsWith(GifConstants.XmpApplicationIdentificationBytes); + if (isXmp) + { + this.ReadXmpApplicationExtension(stream, position, appLength); + return; + } - int subBlockSize = stream.ReadByte(); + int subBlockSize = stream.ReadByte(); + if (subBlockSize == -1) + { + GifThrowHelper.ThrowInvalidImageContentException("Unexpected end of stream while reading gif application extension"); + } - // TODO: There's also a NETSCAPE buffer extension. - // http://www.vurdalakov.net/misc/gif/netscape-buffering-application-extension - if (subBlockSize == GifConstants.NetscapeLoopingSubBlockSize) - { - stream.Read(this.buffer.Span, 0, GifConstants.NetscapeLoopingSubBlockSize); - this.gifMetadata!.RepeatCount = GifNetscapeLoopingApplicationExtension.Parse(this.buffer.Span[1..]).RepeatCount; - stream.Skip(1); // Skip the terminator. - return; - } + // TODO: There's also a NETSCAPE buffer extension. + // http://www.vurdalakov.net/misc/gif/netscape-buffering-application-extension + if (subBlockSize == GifConstants.NetscapeLoopingSubBlockSize) + { + this.ReadNetscapeApplicationExtension(stream); + return; + } + + // Could be something else not supported yet. + // Skip the subblock and terminator. + SkipBlock(stream, subBlockSize); + } + + /// + /// Reads the GIF XMP application extension. + /// + /// The containing image data. + /// The stream position where the application identifier begins. + /// The application block length. + private void ReadXmpApplicationExtension(BufferedReadStream stream, long applicationPosition, int appLength) + { + if (this.skipMetadata) + { + stream.Position = applicationPosition; + SkipBlock(stream, appLength); + return; + } - // Could be something else not supported yet. - // Skip the subblock and terminator. - SkipBlock(stream, subBlockSize); + bool completed = false; + this.ExecuteAncillarySegmentAction( + () => + { + this.ReadXmpApplicationExtensionData(stream, applicationPosition, appLength); + completed = true; + }); + + if (!completed) + { + stream.Position = applicationPosition; + SkipBlock(stream, appLength); + } + } + /// + /// Reads the GIF XMP application extension data. + /// + /// The containing image data. + /// The stream position where the application identifier begins. + /// The application block length. + private void ReadXmpApplicationExtensionData(BufferedReadStream stream, long applicationPosition, int appLength) + { + GifXmpApplicationExtension extension = GifXmpApplicationExtension.Read(stream, this.memoryAllocator); + if (extension.Data.Length > 0) + { + this.metadata!.XmpProfile = new XmpProfile(extension.Data); return; } - SkipBlock(stream, appLength); // Not supported by any known decoder. + stream.Position = applicationPosition; + SkipBlock(stream, appLength); + } + + /// + /// Reads the GIF NETSCAPE looping application extension. + /// + /// The containing image data. + private void ReadNetscapeApplicationExtension(BufferedReadStream stream) => + this.ExecuteAncillarySegmentAction(() => this.ReadNetscapeApplicationExtensionData(stream)); + + /// + /// Reads the GIF NETSCAPE looping application extension data. + /// + /// The containing image data. + private void ReadNetscapeApplicationExtensionData(BufferedReadStream stream) + { + int bytesRead = stream.Read(this.buffer.Span, 0, GifConstants.NetscapeLoopingSubBlockSize); + if (bytesRead != GifConstants.NetscapeLoopingSubBlockSize) + { + throw new InvalidImageContentException("Unexpected end of stream while reading gif application extension"); + } + + this.gifMetadata!.RepeatCount = GifNetscapeLoopingApplicationExtension.Parse(this.buffer.Span[1..]).RepeatCount; + + int terminator = stream.ReadByte(); + if (terminator == -1) + { + throw new InvalidImageContentException("Unexpected end of stream while reading gif application extension"); + } } /// @@ -428,7 +512,12 @@ private void ReadComments(BufferedReadStream stream) using IMemoryOwner commentsBuffer = this.memoryAllocator.Allocate(length); Span commentsSpan = commentsBuffer.GetSpan(); - stream.Read(commentsSpan); + int bytesRead = stream.Read(commentsSpan); + if (bytesRead != length) + { + GifThrowHelper.ThrowInvalidImageContentException("Unexpected end of stream while reading gif comment"); + } + string commentPart = GifConstants.Encoding.GetString(commentsSpan); stringBuilder.Append(commentPart); } diff --git a/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs index dc78f28bf1..85c4959a98 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs @@ -30,7 +30,11 @@ namespace SixLabors.ImageSharp.Formats.Gif; /// The XMP metadata public static GifXmpApplicationExtension Read(Stream stream, MemoryAllocator allocator) { - byte[] xmpBytes = ReadXmpData(stream, allocator); + byte[] xmpBytes = ReadXmpData(stream, allocator, out bool terminated); + if (!terminated) + { + throw new InvalidImageContentException("Unexpected end of stream while reading gif XMP data"); + } // Exclude the "magic trailer", see XMP Specification Part 3, 1.1.2 GIF int xmpLength = xmpBytes.Length - 256; // 257 - unread 0x0 @@ -71,7 +75,7 @@ public int WriteTo(Span buffer) return this.ContentLength; } - private static byte[] ReadXmpData(Stream stream, MemoryAllocator allocator) + private static byte[] ReadXmpData(Stream stream, MemoryAllocator allocator, out bool terminated) { using ChunkedMemoryStream bytes = new(allocator); @@ -83,8 +87,15 @@ private static byte[] ReadXmpData(Stream stream, MemoryAllocator allocator) while (true) { int b = stream.ReadByte(); - if (b <= 0) + if (b == 0) + { + terminated = true; + return bytes.ToArray(); + } + + if (b < 0) { + terminated = false; return bytes.ToArray(); } diff --git a/src/ImageSharp/Formats/ImageDecoderCore.cs b/src/ImageSharp/Formats/ImageDecoderCore.cs index f6b1a92a03..23b517568d 100644 --- a/src/ImageSharp/Formats/ImageDecoderCore.cs +++ b/src/ImageSharp/Formats/ImageDecoderCore.cs @@ -33,6 +33,74 @@ protected ImageDecoderCore(DecoderOptions options) /// public Size Dimensions { get; protected internal set; } + /// + /// Executes a known ancillary segment parsing action using the configured integrity policy. + /// + /// The action. + protected void ExecuteAncillarySegmentAction(Action action) + { + if (this.Options.SegmentIntegrityHandling is SegmentIntegrityHandling.Strict) + { + action(); + return; + } + + try + { + action(); + } + catch (Exception ex) when (ex + is ImageFormatException + or InvalidIccProfileException + or InvalidImageContentException + or InvalidOperationException + or NotSupportedException) + { + // Intentionally ignored in non-strict segment integrity modes. + } + } + + /// + /// Executes a known image data segment parsing action using the configured integrity policy. + /// + /// The action. + protected void ExecuteImageDataSegmentAction(Action action) + { + if (this.Options.SegmentIntegrityHandling is not SegmentIntegrityHandling.IgnoreImageData) + { + action(); + return; + } + + try + { + action(); + } + catch (Exception ex) when (ex + is ImageFormatException + or InvalidIccProfileException + or InvalidImageContentException + or InvalidOperationException + or NotSupportedException) + { + // Intentionally ignored when image data integrity handling is set to IgnoreImageData. + } + } + + /// + /// Throws unless the decoder is running in a non-strict segment integrity mode. + /// Use this only from within when local control flow + /// must continue after the error. + /// + /// The exception message. + protected void ThrowOrIgnoreNonStrictSegmentError(string message) + { + if (this.Options.SegmentIntegrityHandling is SegmentIntegrityHandling.Strict) + { + throw new InvalidImageContentException(message); + } + } + /// /// Reads the raw image information from the specified stream. /// diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index d4517e9f19..ae26914e8e 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -208,11 +208,7 @@ protected override Image Decode(BufferedReadStream stream, Cance JpegThrowHelper.ThrowInvalidImageContentException("Missing SOS marker."); } - this.InitExifProfile(); - this.InitIccProfile(); - this.InitIptcProfile(); - this.InitXmpProfile(); - this.InitDerivedMetadataProperties(); + this.InitializeMetadataProfiles(); _ = this.Options.TryGetIccProfileForColorConversion(this.Metadata.IccProfile, out IccProfile profile); @@ -232,11 +228,7 @@ protected override ImageInfo Identify(BufferedReadStream stream, CancellationTok JpegThrowHelper.ThrowInvalidImageContentException("Missing SOS marker."); } - this.InitExifProfile(); - this.InitIccProfile(); - this.InitIptcProfile(); - this.InitXmpProfile(); - this.InitDerivedMetadataProperties(); + this.InitializeMetadataProfiles(); Size pixelSize = this.Frame.PixelSize; return new ImageInfo(new Size(pixelSize.Width, pixelSize.Height), this.Metadata); @@ -385,7 +377,11 @@ internal void ParseStream(BufferedReadStream stream, SpectralConverter spectralC case JpegConstants.Markers.SOF1: case JpegConstants.Markers.SOF2: - this.ProcessStartOfFrameMarker(stream, markerContentByteSize, fileMarker, ComponentType.Huffman, metadataOnly); + if (!this.ProcessStartOfFrameMarker(stream, markerContentByteSize, fileMarker, ComponentType.Huffman, metadataOnly)) + { + return; + } + break; case JpegConstants.Markers.SOF9: @@ -398,7 +394,11 @@ internal void ParseStream(BufferedReadStream stream, SpectralConverter spectralC this.scanDecoder.ResetInterval = this.resetInterval.Value; } - this.ProcessStartOfFrameMarker(stream, markerContentByteSize, fileMarker, ComponentType.Arithmetic, metadataOnly); + if (!this.ProcessStartOfFrameMarker(stream, markerContentByteSize, fileMarker, ComponentType.Arithmetic, metadataOnly)) + { + return; + } + break; case JpegConstants.Markers.SOF5: @@ -429,7 +429,9 @@ internal void ParseStream(BufferedReadStream stream, SpectralConverter spectralC } // It's highly unlikely that APPn related data will be found after the SOS marker - // We should have gathered everything we need by now. + // So we can stop parsing here and return the metadata we have parsed so far, instead + // of trying to parse any APPn markers after the SOS marker and risking running out of + // memory or other exceptions. return; case JpegConstants.Markers.DHT: @@ -701,6 +703,10 @@ private void InitIccProfile() { this.Metadata.IccProfile = profile; } + else + { + throw new InvalidIccProfileException("Invalid ICC profile."); + } } } @@ -771,6 +777,18 @@ private double GetExifResolutionValue(ExifTag tag) return 0; } + /// + /// Initializes decoded metadata profiles using the configured ancillary segment handling policy. + /// + private void InitializeMetadataProfiles() + { + this.ExecuteAncillarySegmentAction(this.InitExifProfile); + this.ExecuteAncillarySegmentAction(this.InitIccProfile); + this.ExecuteAncillarySegmentAction(this.InitIptcProfile); + this.ExecuteAncillarySegmentAction(this.InitXmpProfile); + this.ExecuteAncillarySegmentAction(this.InitDerivedMetadataProperties); + } + /// /// Extends the profile with additional data. /// @@ -794,7 +812,16 @@ private void ProcessApplicationHeaderMarker(BufferedReadStream stream, int remai // We can only decode JFif identifiers. // Some images contain multiple JFIF markers (Issue 1932) so we check to see // if it's already been read. - if (remaining < JFifMarker.Length || (!this.jFif.Equals(default))) + if (remaining < JFifMarker.Length) + { + this.ThrowOrIgnoreNonStrictSegmentError("Bad App0 Marker length."); + + // Skip the application header length + stream.Skip(remaining); + return; + } + + if (!this.jFif.Equals(default)) { // Skip the application header length stream.Skip(remaining); @@ -804,7 +831,10 @@ private void ProcessApplicationHeaderMarker(BufferedReadStream stream, int remai Span temp = stackalloc byte[2 * 16 * 4]; stream.Read(temp, 0, JFifMarker.Length); - _ = JFifMarker.TryParse(temp, out this.jFif); + if (!JFifMarker.TryParse(temp, out this.jFif)) + { + this.ThrowOrIgnoreNonStrictSegmentError("Invalid App0 marker."); + } remaining -= JFifMarker.Length; @@ -813,7 +843,9 @@ private void ProcessApplicationHeaderMarker(BufferedReadStream stream, int remai { if (stream.Position + remaining >= stream.Length) { - JpegThrowHelper.ThrowInvalidImageContentException("Bad App0 Marker length."); + this.ThrowOrIgnoreNonStrictSegmentError("Bad App0 Marker length."); + stream.Skip(remaining); + return; } stream.Skip(remaining); @@ -829,7 +861,16 @@ private void ProcessApp1Marker(BufferedReadStream stream, int remaining) { const int exifMarkerLength = 6; const int xmpMarkerLength = 29; - if (remaining < exifMarkerLength || this.skipMetadata) + if (remaining < exifMarkerLength) + { + this.ThrowOrIgnoreNonStrictSegmentError("Bad App1 Marker length."); + + // Skip the application header length. + stream.Skip(remaining); + return; + } + + if (this.skipMetadata) { // Skip the application header length. stream.Skip(remaining); @@ -838,7 +879,9 @@ private void ProcessApp1Marker(BufferedReadStream stream, int remaining) if (stream.Position + remaining >= stream.Length) { - JpegThrowHelper.ThrowInvalidImageContentException("Bad App1 Marker length."); + this.ThrowOrIgnoreNonStrictSegmentError("Bad App1 Marker length."); + stream.Skip(remaining); + return; } Span temp = stackalloc byte[2 * 16 * 4]; @@ -869,8 +912,10 @@ private void ProcessApp1Marker(BufferedReadStream stream, int remaining) if (ProfileResolver.IsProfile(temp, ProfileResolver.XmpMarker[..exifMarkerLength])) { const int remainingXmpMarkerBytes = xmpMarkerLength - exifMarkerLength; - if (remaining < remainingXmpMarkerBytes || this.skipMetadata) + if (remaining < remainingXmpMarkerBytes) { + this.ThrowOrIgnoreNonStrictSegmentError("Bad App1 Marker length."); + // Skip the application header length. stream.Skip(remaining); return; @@ -896,6 +941,10 @@ private void ProcessApp1Marker(BufferedReadStream stream, int remaining) remaining = 0; } + else + { + this.ThrowOrIgnoreNonStrictSegmentError("Invalid App1 marker."); + } } // Skip over any remaining bytes of this header. @@ -911,7 +960,15 @@ private void ProcessApp2Marker(BufferedReadStream stream, int remaining) { // Length is 14 though we only need to check 12. const int icclength = 14; - if (remaining < icclength || this.skipMetadata) + if (remaining < icclength) + { + this.ThrowOrIgnoreNonStrictSegmentError("Bad App2 Marker length."); + + stream.Skip(remaining); + return; + } + + if (this.skipMetadata) { stream.Skip(remaining); return; @@ -952,7 +1009,15 @@ private void ProcessApp2Marker(BufferedReadStream stream, int remaining) /// The remaining bytes in the segment block. private void ProcessApp13Marker(BufferedReadStream stream, int remaining) { - if (remaining < ProfileResolver.AdobePhotoshopApp13Marker.Length || this.skipMetadata) + if (remaining < ProfileResolver.AdobePhotoshopApp13Marker.Length) + { + this.ThrowOrIgnoreNonStrictSegmentError("Bad App13 Marker length."); + + stream.Skip(remaining); + return; + } + + if (this.skipMetadata) { stream.Skip(remaining); return; @@ -970,6 +1035,7 @@ private void ProcessApp13Marker(BufferedReadStream stream, int remaining) { if (!ProfileResolver.IsProfile(blockDataSpan[..4], ProfileResolver.AdobeImageResourceBlockMarker)) { + this.ThrowOrIgnoreNonStrictSegmentError("Invalid App13 marker."); return; } @@ -986,6 +1052,9 @@ private void ProcessApp13Marker(BufferedReadStream stream, int remaining) this.iptcData = blockDataSpan.Slice(dataStartIdx, resourceDataSize).ToArray(); break; } + + this.ThrowOrIgnoreNonStrictSegmentError("Invalid App13 marker."); + return; } else { @@ -995,6 +1064,7 @@ private void ProcessApp13Marker(BufferedReadStream stream, int remaining) if (blockDataSpan.Length < dataStartIdx + resourceDataSize) { // Not enough data or the resource data size is wrong. + this.ThrowOrIgnoreNonStrictSegmentError("Invalid App13 marker."); break; } @@ -1089,6 +1159,8 @@ private void ProcessApp14Marker(BufferedReadStream stream, int remaining) const int markerLength = AdobeMarker.Length; if (remaining < markerLength) { + this.ThrowOrIgnoreNonStrictSegmentError("Bad App14 Marker length."); + // Skip the application header length stream.Skip(remaining); return; @@ -1103,6 +1175,10 @@ private void ProcessApp14Marker(BufferedReadStream stream, int remaining) { this.hasAdobeMarker = true; } + else + { + this.ThrowOrIgnoreNonStrictSegmentError("Invalid App14 marker."); + } if (remaining > 0) { @@ -1212,13 +1288,19 @@ private void ProcessDefineQuantizationTablesMarker(BufferedReadStream stream, in /// The current frame marker. /// The jpeg decoding component type. /// Whether to parse metadata only. - private void ProcessStartOfFrameMarker(BufferedReadStream stream, int remaining, in JpegFileMarker frameMarker, ComponentType decodingComponentType, bool metadataOnly) + private bool ProcessStartOfFrameMarker(BufferedReadStream stream, int remaining, in JpegFileMarker frameMarker, ComponentType decodingComponentType, bool metadataOnly) { if (this.Frame != null) { - if (metadataOnly) + // If we have found the SOS marker, we can stop parsing as we have all + // the information we need to decode the image. + // It's possible that there are APPn related markers after the SOS marker, + // but it's highly unlikely and we would be better off stopping parsing + // and decoding the image instead of trying to parse those APPn markers + // and risking running out of memory or other exceptions. + if (this.hasSOSMarker) { - return; + return false; } JpegThrowHelper.ThrowInvalidImageContentException("Multiple SOF markers. Only single frame jpegs supported."); @@ -1351,6 +1433,8 @@ private void ProcessStartOfFrameMarker(BufferedReadStream stream, int remaining, this.Frame.Init(maxH, maxV); this.scanDecoder.InjectFrameData(this.Frame, this); } + + return true; } /// @@ -1550,7 +1634,7 @@ private void ProcessStartOfScanMarker(BufferedReadStream stream, int remaining) arithmeticScanDecoder.InitDecodingTables(this.arithmeticDecodingTables); } - this.InitIccProfile(); + this.ExecuteAncillarySegmentAction(this.InitIccProfile); _ = this.Options.TryGetIccProfileForColorConversion(this.Metadata.IccProfile, out IccProfile profile); this.scanDecoder.ParseEntropyCodedData(selectorsCount, profile); } diff --git a/src/ImageSharp/Formats/Png/PngChunk.cs b/src/ImageSharp/Formats/Png/PngChunk.cs index 3883986d06..bbf1a88bf2 100644 --- a/src/ImageSharp/Formats/Png/PngChunk.cs +++ b/src/ImageSharp/Formats/Png/PngChunk.cs @@ -43,11 +43,11 @@ public PngChunk(int length, PngChunkType type, IMemoryOwner data = null) /// /// The segment handling behavior. public bool IsCritical(SegmentIntegrityHandling handling) - => handling switch + => this.Type switch { - SegmentIntegrityHandling.IgnoreNone => true, - SegmentIntegrityHandling.IgnoreNonCritical => this.Type is PngChunkType.Header or PngChunkType.Palette or PngChunkType.Data or PngChunkType.FrameData, - SegmentIntegrityHandling.IgnoreData => this.Type is PngChunkType.Header or PngChunkType.Palette, - _ => false, + PngChunkType.Header => true, + PngChunkType.Palette => true, + PngChunkType.Data or PngChunkType.FrameData => handling < SegmentIntegrityHandling.IgnoreImageData, + _ => handling < SegmentIntegrityHandling.IgnoreAncillary, }; } diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index d794c66e27..5b9eee1169 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -800,18 +800,46 @@ private void DecodePixelData( PngMetadata pngMetadata, CancellationToken cancellationToken) where TPixel : unmanaged, IPixel + { + using IMemoryOwner? blendMemory = frameControl.BlendMode == FrameBlendMode.Over + ? this.memoryAllocator.Allocate(imageFrame.Width, AllocationOptions.Clean) + : null; + + this.ExecuteImageDataSegmentAction(() => this.DecodePixelDataCore( + frameControl, + compressedStream, + imageFrame, + pngMetadata, + blendMemory, + cancellationToken)); + + this.hasImageData = true; + } + + /// + /// Decodes the raw pixel data row by row. + /// + /// The pixel format. + /// The frame control. + /// The compressed pixel data stream. + /// The image frame to decode to. + /// The png metadata. + /// The optional row blending buffer. + /// The cancellation token. + private void DecodePixelDataCore( + FrameControl frameControl, + DeflateStream compressedStream, + ImageFrame imageFrame, + PngMetadata pngMetadata, + IMemoryOwner? blendMemory, + CancellationToken cancellationToken) + where TPixel : unmanaged, IPixel { int currentRow = (int)frameControl.YOffset; int currentRowBytesRead = 0; int height = (int)frameControl.YMax; - IMemoryOwner? blendMemory = null; - Span blendRowBuffer = []; - if (frameControl.BlendMode == FrameBlendMode.Over) - { - blendMemory = this.memoryAllocator.Allocate(imageFrame.Width, AllocationOptions.Clean); - blendRowBuffer = blendMemory.Memory.Span; - } + Span blendRowBuffer = blendMemory is null ? [] : blendMemory.Memory.Span; while (currentRow < height) { @@ -855,11 +883,6 @@ private void DecodePixelData( break; default: - if (this.segmentIntegrityHandling is SegmentIntegrityHandling.IgnoreData or SegmentIntegrityHandling.IgnoreAll) - { - goto EXIT; - } - PngThrowHelper.ThrowUnknownFilter(); break; } @@ -870,8 +893,7 @@ private void DecodePixelData( } EXIT: - this.hasImageData = true; - blendMemory?.Dispose(); + return; } /// @@ -890,6 +912,41 @@ private void DecodeInterlacedPixelData( PngMetadata pngMetadata, CancellationToken cancellationToken) where TPixel : unmanaged, IPixel + { + using IMemoryOwner? blendMemory = frameControl.BlendMode == FrameBlendMode.Over + ? this.memoryAllocator.Allocate(imageFrame.Width, AllocationOptions.Clean) + : null; + + FrameControl frameControlCopy = frameControl; + this.ExecuteImageDataSegmentAction(() => this.DecodeInterlacedPixelDataCore( + frameControlCopy, + compressedStream, + imageFrame, + pngMetadata, + blendMemory, + cancellationToken)); + + this.hasImageData = true; + } + + /// + /// Decodes the raw interlaced pixel data row by row. + /// + /// The pixel format. + /// The frame control. + /// The compressed pixel data stream. + /// The current image frame. + /// The png metadata. + /// The optional row blending buffer. + /// The cancellation token. + private void DecodeInterlacedPixelDataCore( + FrameControl frameControl, + DeflateStream compressedStream, + ImageFrame imageFrame, + PngMetadata pngMetadata, + IMemoryOwner? blendMemory, + CancellationToken cancellationToken) + where TPixel : unmanaged, IPixel { int currentRow = Adam7.FirstRow[0] + (int)frameControl.YOffset; int currentRowBytesRead = 0; @@ -899,13 +956,7 @@ private void DecodeInterlacedPixelData( Buffer2D imageBuffer = imageFrame.PixelBuffer; - IMemoryOwner? blendMemory = null; - Span blendRowBuffer = []; - if (frameControl.BlendMode == FrameBlendMode.Over) - { - blendMemory = this.memoryAllocator.Allocate(imageFrame.Width, AllocationOptions.Clean); - blendRowBuffer = blendMemory.Memory.Span; - } + Span blendRowBuffer = blendMemory is null ? [] : blendMemory.Memory.Span; while (true) { @@ -962,11 +1013,6 @@ private void DecodeInterlacedPixelData( break; default: - if (this.segmentIntegrityHandling is SegmentIntegrityHandling.IgnoreData or SegmentIntegrityHandling.IgnoreAll) - { - goto EXIT; - } - PngThrowHelper.ThrowUnknownFilter(); break; } @@ -1002,8 +1048,7 @@ private void DecodeInterlacedPixelData( } EXIT: - this.hasImageData = true; - blendMemory?.Dispose(); + return; } /// diff --git a/src/ImageSharp/Formats/SegmentIntegrityHandling.cs b/src/ImageSharp/Formats/SegmentIntegrityHandling.cs index 977aee4ad5..f16c1bb261 100644 --- a/src/ImageSharp/Formats/SegmentIntegrityHandling.cs +++ b/src/ImageSharp/Formats/SegmentIntegrityHandling.cs @@ -1,30 +1,26 @@ -// Copyright (c) Six Labors. +// Copyright (c) Six Labors. // Licensed under the Six Labors Split License. namespace SixLabors.ImageSharp.Formats; /// -/// Specifies how to handle validation of errors in different segments of encoded image files. +/// Specifies how to handle validation of recoverable errors in ancillary and image data segments. +/// Structural errors that prevent safe decoding remain fatal regardless of the selected mode. /// public enum SegmentIntegrityHandling { /// - /// Do not ignore any errors. + /// Do not ignore any recoverable ancillary or image data segment errors. /// - IgnoreNone, + Strict = 0, /// - /// Ignore errors in non-critical segments of the encoded image. + /// Ignore recoverable errors in ancillary segments, such as optional metadata. /// - IgnoreNonCritical, + IgnoreAncillary = 1, /// - /// Ignore errors in data segments (e.g., image data, metadata). + /// Ignore recoverable errors in image data segments in addition to ancillary segments. /// - IgnoreData, - - /// - /// Ignore errors in all segments. - /// - IgnoreAll + IgnoreImageData = 2, } diff --git a/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs b/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs index 1822cb8d15..e232045bee 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs @@ -289,7 +289,19 @@ private ImageFrame DecodeFrame(ExifProfile tags, Size? size, Can // We resolve the ICC profile early so that we can use it for color conversion if needed. if (tags.TryGetValue(ExifTag.IccProfile, out IExifValue iccProfileBytes)) { - imageFrameMetaData.IccProfile = new IccProfile(iccProfileBytes.Value); + this.ExecuteAncillarySegmentAction( + () => + { + IccProfile profile = new(iccProfileBytes.Value); + if (profile.CheckIsValid()) + { + imageFrameMetaData.IccProfile = profile; + } + else + { + throw new InvalidIccProfileException("Invalid TIFF ICC profile."); + } + }); } } diff --git a/src/ImageSharp/Formats/Webp/WebpAnimationDecoder.cs b/src/ImageSharp/Formats/Webp/WebpAnimationDecoder.cs index 71e609fbcb..8a8ad823dc 100644 --- a/src/ImageSharp/Formats/Webp/WebpAnimationDecoder.cs +++ b/src/ImageSharp/Formats/Webp/WebpAnimationDecoder.cs @@ -64,9 +64,9 @@ internal class WebpAnimationDecoder : IDisposable private readonly BackgroundColorHandling backgroundColorHandling; /// - /// How to handle validation of errors in different segments of encoded image files. + /// Executes a known ancillary segment parsing action using the configured integrity policy. /// - private readonly SegmentIntegrityHandling segmentIntegrityHandling; + private readonly Action executeAncillarySegmentAction; /// /// Initializes a new instance of the class. @@ -76,21 +76,21 @@ internal class WebpAnimationDecoder : IDisposable /// The maximum number of frames to decode. Inclusive. /// Whether to skip metadata. /// The flag to decide how to handle the background color in the Animation Chunk. - /// How to handle validation of errors in different segments of encoded image files. + /// Executes a known ancillary segment parsing action using the configured integrity policy. public WebpAnimationDecoder( MemoryAllocator memoryAllocator, Configuration configuration, uint maxFrames, bool skipMetadata, BackgroundColorHandling backgroundColorHandling, - SegmentIntegrityHandling segmentIntegrityHandling) + Action executeAncillarySegmentAction) { this.memoryAllocator = memoryAllocator; this.configuration = configuration; this.maxFrames = maxFrames; this.skipMetadata = skipMetadata; this.backgroundColorHandling = backgroundColorHandling; - this.segmentIntegrityHandling = segmentIntegrityHandling; + this.executeAncillarySegmentAction = executeAncillarySegmentAction; } /// @@ -118,7 +118,6 @@ public ImageInfo Identify( : features.AnimationBackgroundColor!.Value; bool ignoreMetadata = this.skipMetadata; - SegmentIntegrityHandling segmentIntegrityHandling = this.segmentIntegrityHandling; Span buffer = stackalloc byte[4]; uint frameCount = 0; int remainingBytes = (int)completeDataSize; @@ -139,13 +138,7 @@ public ImageInfo Identify( case WebpChunkType.Iccp: case WebpChunkType.Xmp: case WebpChunkType.Exif: - WebpChunkParsingUtils.ParseOptionalChunks( - stream, - chunkType, - this.metadata, - ignoreMetadata, - segmentIntegrityHandling, - buffer); + this.ReadOptionalChunk(stream, chunkType, this.metadata, ignoreMetadata); break; default: @@ -196,7 +189,6 @@ public Image Decode( TPixel backgroundPixel = backgroundColor.ToPixel(); bool ignoreMetadata = this.skipMetadata; - SegmentIntegrityHandling segmentIntegrityHandling = this.segmentIntegrityHandling; Span buffer = stackalloc byte[4]; uint frameCount = 0; int remainingBytes = (int)completeDataSize; @@ -223,7 +215,7 @@ public Image Decode( case WebpChunkType.Iccp: case WebpChunkType.Xmp: case WebpChunkType.Exif: - WebpChunkParsingUtils.ParseOptionalChunks(stream, chunkType, image!.Metadata, ignoreMetadata, segmentIntegrityHandling, buffer); + this.ReadOptionalChunk(stream, chunkType, image!.Metadata, ignoreMetadata); break; default: @@ -380,6 +372,29 @@ private static void SetFrameMetadata(ImageFrameMetadata meta, WebpFrameData fram frameMetadata.DisposalMode = frameData.DisposalMethod; } + private void ReadOptionalChunk( + BufferedReadStream stream, + WebpChunkType chunkType, + ImageMetadata imageMetadata, + bool ignoreMetadata) + { + switch (chunkType) + { + case WebpChunkType.Iccp: + + // While ICC profiles are optional, an invalid ICC profile cannot be ignored because it must + // precede the frame data, and we cannot safely skip it without successfully reading its size. + WebpChunkParsingUtils.ReadIccProfile(stream, imageMetadata, ignoreMetadata); + break; + case WebpChunkType.Exif: + this.executeAncillarySegmentAction(() => WebpChunkParsingUtils.ReadExifProfile(stream, imageMetadata, ignoreMetadata)); + break; + case WebpChunkType.Xmp: + this.executeAncillarySegmentAction(() => WebpChunkParsingUtils.ReadXmpProfile(stream, imageMetadata, ignoreMetadata)); + break; + } + } + /// /// Reads the ALPH chunk data. /// diff --git a/src/ImageSharp/Formats/Webp/WebpChunkParsingUtils.cs b/src/ImageSharp/Formats/Webp/WebpChunkParsingUtils.cs index 26ae28fd4c..119d53fe96 100644 --- a/src/ImageSharp/Formats/Webp/WebpChunkParsingUtils.cs +++ b/src/ImageSharp/Formats/Webp/WebpChunkParsingUtils.cs @@ -343,69 +343,18 @@ public static WebpChunkType ReadChunkType(BufferedReadStream stream, Span throw new ImageFormatException("Invalid Webp data, could not read chunk type."); } - /// - /// Parses optional metadata chunks. There SHOULD be at most one chunk of each type ('EXIF' and 'XMP '). - /// If there are more such chunks, readers MAY ignore all except the first one. - /// Also, a file may possibly contain both 'EXIF' and 'XMP ' chunks. - /// - /// The stream to read the data from. - /// The chunk type to parse. - /// The image metadata to write to. - /// If true, metadata will be ignored. - /// Indicates how to handle segment integrity issues. - /// Buffer to store the data read from the stream. - public static void ParseOptionalChunks( - BufferedReadStream stream, - WebpChunkType chunkType, - ImageMetadata metadata, - bool ignoreMetadata, - SegmentIntegrityHandling segmentIntegrityHandling, - Span buffer) - { - long streamLength = stream.Length; - while (stream.Position < streamLength) - { - switch (chunkType) - { - case WebpChunkType.Iccp: - ReadIccProfile(stream, metadata, ignoreMetadata, segmentIntegrityHandling, buffer); - break; - - case WebpChunkType.Exif: - ReadExifProfile(stream, metadata, ignoreMetadata, segmentIntegrityHandling, buffer); - break; - case WebpChunkType.Xmp: - ReadXmpProfile(stream, metadata, ignoreMetadata, segmentIntegrityHandling, buffer); - break; - default: - - // Ignore unknown chunks. - // These must always fall after the image data so we are safe to always skip them. - uint chunkLength = ReadChunkSize(stream, buffer, false); - stream.Skip((int)chunkLength); - break; - } - } - } - /// /// Reads the ICCP chunk from the stream. /// /// The stream to decode from. /// The image metadata. /// If true, metadata will be ignored. - /// Indicates how to handle segment integrity issues. - /// Temporary buffer. public static void ReadIccProfile( BufferedReadStream stream, ImageMetadata metadata, - bool ignoreMetadata, - SegmentIntegrityHandling segmentIntegrityHandling, - Span buffer) + bool ignoreMetadata) { - // While ICC profiles are optional, an invalid ICC profile cannot be ignored as it must precede the image data - // and since we canot determine its size to allow skipping without reading the chunk size, we have to throw if it's invalid. - // Hence we do not consider segment integrity handling here. + Span buffer = stackalloc byte[4]; uint iccpChunkSize = ReadChunkSize(stream, buffer); if (ignoreMetadata || metadata.IccProfile != null) { @@ -413,22 +362,20 @@ public static void ReadIccProfile( } else { - bool ignoreNone = segmentIntegrityHandling == SegmentIntegrityHandling.IgnoreNone; byte[] iccpData = new byte[iccpChunkSize]; int bytesRead = stream.Read(iccpData, 0, (int)iccpChunkSize); - - // We have the size but the profile is invalid if we cannot read enough data. - // Use the segment integrity handling to determine if we throw. - if (bytesRead != iccpChunkSize && ignoreNone) + if (bytesRead != iccpChunkSize) { WebpThrowHelper.ThrowInvalidImageContentException("Not enough data to read the iccp chunk"); } IccProfile profile = new(iccpData); - if (profile.CheckIsValid()) + if (!profile.CheckIsValid()) { - metadata.IccProfile = profile; + throw new InvalidIccProfileException("Invalid ICC profile."); } + + metadata.IccProfile = profile; } } @@ -438,17 +385,13 @@ public static void ReadIccProfile( /// The stream to decode from. /// The image metadata. /// If true, metadata will be ignored. - /// Indicates how to handle segment integrity issues. - /// Temporary buffer. public static void ReadExifProfile( BufferedReadStream stream, ImageMetadata metadata, - bool ignoreMetadata, - SegmentIntegrityHandling segmentIntegrityHandling, - Span buffer) + bool ignoreMetadata) { - bool ignoreNone = segmentIntegrityHandling == SegmentIntegrityHandling.IgnoreNone; - uint exifChunkSize = ReadChunkSize(stream, buffer, ignoreNone); + Span buffer = stackalloc byte[4]; + uint exifChunkSize = ReadChunkSize(stream, buffer); if (ignoreMetadata || metadata.ExifProfile != null) { stream.Skip((int)exifChunkSize); @@ -459,12 +402,7 @@ public static void ReadExifProfile( int bytesRead = stream.Read(exifData, 0, (int)exifChunkSize); if (bytesRead != exifChunkSize) { - if (ignoreNone) - { - WebpThrowHelper.ThrowImageFormatException("Could not read enough data for the EXIF profile"); - } - - return; + WebpThrowHelper.ThrowInvalidImageContentException("Could not read enough data for the EXIF profile"); } ExifProfile exifProfile = new(exifData); @@ -490,18 +428,13 @@ public static void ReadExifProfile( /// The stream to decode from. /// The image metadata. /// If true, metadata will be ignored. - /// Indicates how to handle segment integrity issues. - /// Temporary buffer. public static void ReadXmpProfile( BufferedReadStream stream, ImageMetadata metadata, - bool ignoreMetadata, - SegmentIntegrityHandling segmentIntegrityHandling, - Span buffer) + bool ignoreMetadata) { - bool ignoreNone = segmentIntegrityHandling == SegmentIntegrityHandling.IgnoreNone; - - uint xmpChunkSize = ReadChunkSize(stream, buffer, ignoreNone); + Span buffer = stackalloc byte[4]; + uint xmpChunkSize = ReadChunkSize(stream, buffer); if (ignoreMetadata || metadata.XmpProfile != null) { stream.Skip((int)xmpChunkSize); @@ -512,12 +445,7 @@ public static void ReadXmpProfile( int bytesRead = stream.Read(xmpData, 0, (int)xmpChunkSize); if (bytesRead != xmpChunkSize) { - if (ignoreNone) - { - WebpThrowHelper.ThrowImageFormatException("Could not read enough data for the XMP profile"); - } - - return; + WebpThrowHelper.ThrowInvalidImageContentException("Could not read enough data for the XMP profile"); } metadata.XmpProfile = new XmpProfile(xmpData); diff --git a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs index d6a07eecad..55bdca69cb 100644 --- a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs +++ b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs @@ -52,8 +52,6 @@ internal sealed class WebpDecoderCore : ImageDecoderCore, IDisposable /// private readonly BackgroundColorHandling backgroundColorHandling; - private readonly SegmentIntegrityHandling segmentIntegrityHandling; - /// /// Initializes a new instance of the class. /// @@ -62,7 +60,6 @@ public WebpDecoderCore(WebpDecoderOptions options) : base(options.GeneralOptions) { this.backgroundColorHandling = options.BackgroundColorHandling; - this.segmentIntegrityHandling = options.GeneralOptions.SegmentIntegrityHandling; this.configuration = options.GeneralOptions.Configuration; this.skipMetadata = options.GeneralOptions.SkipMetadata; this.maxFrames = options.GeneralOptions.MaxFrames; @@ -90,7 +87,7 @@ protected override Image Decode(BufferedReadStream stream, Cance this.maxFrames, this.skipMetadata, this.backgroundColorHandling, - this.segmentIntegrityHandling); + this.ExecuteAncillarySegmentAction); return animationDecoder.Decode(stream, this.webImageInfo.Features, this.webImageInfo.Width, this.webImageInfo.Height, fileSize); } @@ -149,7 +146,7 @@ protected override ImageInfo Identify(BufferedReadStream stream, CancellationTok this.maxFrames, this.skipMetadata, this.backgroundColorHandling, - this.segmentIntegrityHandling); + this.ExecuteAncillarySegmentAction); return animationDecoder.Identify( stream, @@ -278,19 +275,21 @@ private bool ParseOptionalExtendedChunks( Span buffer) { bool ignoreMetadata = this.skipMetadata; - SegmentIntegrityHandling integrityHandling = this.segmentIntegrityHandling; switch (chunkType) { case WebpChunkType.Iccp: - WebpChunkParsingUtils.ReadIccProfile(stream, metadata, ignoreMetadata, integrityHandling, buffer); + + // While ICC profiles are optional, an invalid ICC profile cannot be ignored because it must + // precede the image data, and we cannot safely skip it without successfully reading its size. + WebpChunkParsingUtils.ReadIccProfile(stream, metadata, ignoreMetadata); break; case WebpChunkType.Exif: - WebpChunkParsingUtils.ReadExifProfile(stream, metadata, ignoreMetadata, integrityHandling, buffer); + this.ExecuteAncillarySegmentAction(() => WebpChunkParsingUtils.ReadExifProfile(stream, metadata, ignoreMetadata)); break; case WebpChunkType.Xmp: - WebpChunkParsingUtils.ReadXmpProfile(stream, metadata, ignoreMetadata, integrityHandling, buffer); + this.ExecuteAncillarySegmentAction(() => WebpChunkParsingUtils.ReadXmpProfile(stream, metadata, ignoreMetadata)); break; case WebpChunkType.AnimationParameter: @@ -320,7 +319,6 @@ private bool ParseOptionalExtendedChunks( private void ParseOptionalChunks(BufferedReadStream stream, ImageMetadata metadata, WebpFeatures features, Span buffer) { bool ignoreMetadata = this.skipMetadata; - SegmentIntegrityHandling integrityHandling = this.segmentIntegrityHandling; if (ignoreMetadata || (!features.ExifProfile && !features.XmpMetaData)) { @@ -334,11 +332,11 @@ private void ParseOptionalChunks(BufferedReadStream stream, ImageMetadata metada WebpChunkType chunkType = WebpChunkParsingUtils.ReadChunkType(stream, buffer); if (chunkType == WebpChunkType.Exif && metadata.ExifProfile == null) { - WebpChunkParsingUtils.ReadExifProfile(stream, metadata, ignoreMetadata, integrityHandling, buffer); + this.ExecuteAncillarySegmentAction(() => WebpChunkParsingUtils.ReadExifProfile(stream, metadata, ignoreMetadata)); } else if (chunkType == WebpChunkType.Xmp && metadata.XmpProfile == null) { - WebpChunkParsingUtils.ReadXmpProfile(stream, metadata, ignoreMetadata, integrityHandling, buffer); + this.ExecuteAncillarySegmentAction(() => WebpChunkParsingUtils.ReadXmpProfile(stream, metadata, ignoreMetadata)); } else { diff --git a/src/ImageSharp/Metadata/Profiles/ICC/TagDataEntries/IccChromaticityTagDataEntry.cs b/src/ImageSharp/Metadata/Profiles/ICC/TagDataEntries/IccChromaticityTagDataEntry.cs index 1938ad6302..0f61857ba7 100644 --- a/src/ImageSharp/Metadata/Profiles/ICC/TagDataEntries/IccChromaticityTagDataEntry.cs +++ b/src/ImageSharp/Metadata/Profiles/ICC/TagDataEntries/IccChromaticityTagDataEntry.cs @@ -140,7 +140,7 @@ private static double[][] GetColorantArray(IccColorantEncoding colorantType) [0.155, 0.070] ]; default: - throw new ArgumentException("Unrecognized colorant encoding"); + throw new InvalidIccProfileException("Unrecognized colorant encoding"); } } diff --git a/tests/ImageSharp.Tests/Formats/Bmp/BmpMetadataTests.cs b/tests/ImageSharp.Tests/Formats/Bmp/BmpMetadataTests.cs index 64564ae1d8..bbfa4ac598 100644 --- a/tests/ImageSharp.Tests/Formats/Bmp/BmpMetadataTests.cs +++ b/tests/ImageSharp.Tests/Formats/Bmp/BmpMetadataTests.cs @@ -1,8 +1,11 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Bmp; +using SixLabors.ImageSharp.Metadata.Profiles.Icc; using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Tests.TestUtilities; using static SixLabors.ImageSharp.Tests.TestImages.Bmp; // ReSharper disable InconsistentNaming @@ -46,7 +49,7 @@ public void Identify_DetectsCorrectBitmapInfoHeaderType(string imagePath, BmpInf } [Theory] - [WithFile(IccProfile, PixelTypes.Rgba32)] + [WithFile(TestImages.Bmp.IccProfile, PixelTypes.Rgba32)] public void Decoder_CanReadColorProfile(TestImageProvider provider) where TPixel : unmanaged, IPixel { @@ -56,4 +59,40 @@ public void Decoder_CanReadColorProfile(TestImageProvider provid Assert.NotNull(metaData.IccProfile); Assert.Equal(16, metaData.IccProfile.Entries.Length); } + + [Fact] + public void Identify_MalformedIccProfile_IgnoresNonCriticalErrorsByDefault() + { + ImageInfo info = Image.Identify(CreateBmpWithMalformedIccProfile()); + Assert.Equal(1, info.Width); + Assert.Equal(1, info.Height); + } + + [Fact] + public void Decode_MalformedIccProfile_IgnoresNonCriticalErrorsByDefault() + { + using Image image = Image.Load(CreateBmpWithMalformedIccProfile()); + Assert.Equal(1, image.Width); + Assert.Equal(1, image.Height); + } + + [Fact] + public void Identify_MalformedIccProfile_ThrowsWithStrict() + { + DecoderOptions options = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.Strict }; + Assert.Throws(() => Image.Identify(options, CreateBmpWithMalformedIccProfile())); + } + + [Fact] + public void Decode_MalformedIccProfile_ThrowsWithStrict() + { + DecoderOptions options = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.Strict }; + Assert.Throws(() => + { + using Image image = Image.Load(options, CreateBmpWithMalformedIccProfile()); + }); + } + + private static byte[] CreateBmpWithMalformedIccProfile() + => CorruptedMetadataImageFactory.CreateImageWithMalformedIccProfile(new BmpEncoder()); } diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index febc65da3e..8a8b4866a2 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -408,4 +408,54 @@ public void Issue2980(TestImageProvider provider) image.DebugSaveMultiFrame(provider); image.CompareToReferenceOutputMultiFrame(provider, ImageComparer.Exact); } + + [Fact] + public void Identify_MalformedApplicationExtension_IgnoresNonCriticalErrorsByDefault() + { + ImageInfo info = Image.Identify(CreateGifWithMalformedApplicationExtension()); + Assert.Equal(1, info.Width); + Assert.Equal(1, info.Height); + } + + [Fact] + public void Decode_MalformedApplicationExtension_IgnoresNonCriticalErrorsByDefault() + { + using Image image = Image.Load(CreateGifWithMalformedApplicationExtension()); + Assert.Equal(1, image.Width); + Assert.Equal(1, image.Height); + } + + [Fact] + public void Identify_MalformedApplicationExtension_ThrowsWithStrict() + { + DecoderOptions options = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.Strict }; + Assert.Throws(() => Image.Identify(options, CreateGifWithMalformedApplicationExtension())); + } + + [Fact] + public void Decode_MalformedApplicationExtension_ThrowsWithStrict() + { + DecoderOptions options = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.Strict }; + Assert.Throws(() => + { + using Image image = Image.Load(options, CreateGifWithMalformedApplicationExtension()); + }); + } + + private static byte[] CreateGifWithMalformedApplicationExtension() + { + // The application extension declares a 5-byte application block (0x05), but GIF application + // extensions require the fixed identifier/authentication block to be 11 bytes long. + return + [ + (byte)'G', (byte)'I', (byte)'F', (byte)'8', (byte)'9', (byte)'a', + 0x01, 0x00, 0x01, 0x00, 0x80, 0x00, 0x00, + 0x00, 0x00, 0x00, 0xFF, 0xFF, 0xFF, + 0x21, 0xFF, 0x05, (byte)'B', (byte)'a', (byte)'d', (byte)'A', (byte)'p', 0x00, + 0x21, 0xF9, 0x04, 0x01, 0x00, 0x00, 0x00, 0x00, + 0x2C, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, + 0x02, 0x02, 0x44, 0x01, 0x00, + 0x3B + ]; + } } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index 36847536b3..2f84a1a7b7 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -457,4 +457,71 @@ public void Issue_3071_Decode_TruncatedJpeg_Throws_InvalidImageContentException( byte[] data = [0xFF, 0xD8, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30]; using Image image = Image.Load(data); }); + + // https://github.com/SixLabors/ImageSharp/issues/3118 + [Theory] + [WithFile(TestImages.Jpeg.Issues.Issue3118, PixelTypes.Rgb24)] + public void Issue3118_Multiple_SOF_WithSOS_DoesNotThrow(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(JpegDecoder.Instance); + image.DebugSave(provider); + } + + [Fact] + public void Identify_MalformedApp13Segment_IgnoresNonCriticalErrorsByDefault() + { + ImageInfo info = Image.Identify(CreateJpegWithMalformedApp13Segment()); + Assert.True(info.Width > 0); + Assert.True(info.Height > 0); + } + + [Fact] + public void Decode_MalformedApp13Segment_IgnoresNonCriticalErrorsByDefault() + { + using Image image = Image.Load(CreateJpegWithMalformedApp13Segment()); + Assert.True(image.Width > 0); + Assert.True(image.Height > 0); + } + + [Fact] + public void Identify_MalformedApp13Segment_ThrowsWithStrict() + { + DecoderOptions options = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.Strict }; + Assert.Throws(() => Image.Identify(options, CreateJpegWithMalformedApp13Segment())); + } + + [Fact] + public void Decode_MalformedApp13Segment_ThrowsWithStrict() + { + DecoderOptions options = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.Strict }; + Assert.Throws(() => + { + using Image image = Image.Load(options, CreateJpegWithMalformedApp13Segment()); + }); + } + + private static byte[] CreateJpegWithMalformedApp13Segment() + { + byte[] source = TestFile.Create(TestImages.Jpeg.Baseline.Calliphora).Bytes; + + // This APP13 segment starts with the valid "Photoshop 3.0\0" identifier, but the remaining + // payload does not begin with the required "8BIM" image resource block signature. + byte[] malformedApp13 = + [ + 0xFF, 0xED, + 0x00, 0x1D, + (byte)'P', (byte)'h', (byte)'o', (byte)'t', (byte)'o', (byte)'s', (byte)'h', (byte)'o', (byte)'p', (byte)' ', (byte)'3', (byte)'.', (byte)'0', 0x00, + (byte)'B', (byte)'a', (byte)'d', (byte)'R', (byte)'e', (byte)'s', (byte)'o', (byte)'u', (byte)'r', (byte)'c', (byte)'e', (byte)'!', (byte)'!' + ]; + + byte[] payload = new byte[source.Length + malformedApp13.Length]; + payload[0] = source[0]; + payload[1] = source[1]; + + Buffer.BlockCopy(malformedApp13, 0, payload, 2, malformedApp13.Length); + Buffer.BlockCopy(source, 2, payload, 2 + malformedApp13.Length, source.Length - 2); + + return payload; + } } diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 4712fc0dd5..803a12b03a 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -405,7 +405,7 @@ public void Identify_IgnoreCrcErrors(string imagePath, int expectedPixelSize) TestFile testFile = TestFile.Create(imagePath); using MemoryStream stream = new(testFile.Bytes, false); - ImageInfo imageInfo = Image.Identify(new DecoderOptions { SegmentIntegrityHandling = SegmentIntegrityHandling.IgnoreData }, stream); + ImageInfo imageInfo = Image.Identify(new DecoderOptions { SegmentIntegrityHandling = SegmentIntegrityHandling.IgnoreImageData }, stream); Assert.NotNull(imageInfo); Assert.Equal(expectedPixelSize, imageInfo.PixelType.BitsPerPixel); @@ -594,7 +594,7 @@ public void Decode_InvalidDataChunkCrc_ThrowsException(TestImageProvider public void Decode_InvalidDataChunkCrc_IgnoreCrcErrors(TestImageProvider provider, bool compare) where TPixel : unmanaged, IPixel { - using Image image = provider.GetImage(PngDecoder.Instance, new DecoderOptions() { SegmentIntegrityHandling = SegmentIntegrityHandling.IgnoreData }); + using Image image = provider.GetImage(PngDecoder.Instance, new DecoderOptions() { SegmentIntegrityHandling = SegmentIntegrityHandling.IgnoreImageData }); image.DebugSave(provider); if (compare) @@ -775,7 +775,7 @@ static void RunTest(string providerDump, string nonContiguousBuffersStr) public void Binary_PrematureEof() { PngDecoder decoder = PngDecoder.Instance; - PngDecoderOptions options = new() { GeneralOptions = new DecoderOptions { SegmentIntegrityHandling = SegmentIntegrityHandling.IgnoreData } }; + PngDecoderOptions options = new() { GeneralOptions = new DecoderOptions { SegmentIntegrityHandling = SegmentIntegrityHandling.IgnoreImageData } }; using EofHitCounter eofHitCounter = EofHitCounter.RunDecoder(TestImages.Png.Bad.FlagOfGermany0000016446, decoder, options); // TODO: Try to reduce this to 1. diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs index 6123d46dac..f98363b7e6 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs @@ -6,7 +6,9 @@ using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Tiff; using SixLabors.ImageSharp.Metadata; +using SixLabors.ImageSharp.Metadata.Profiles.Icc; using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Tests.TestUtilities; using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; using static SixLabors.ImageSharp.Tests.TestImages.Tiff; @@ -868,4 +870,40 @@ public void TiffDecoder_CanDecode_ExtraSamplesUnspecified(TestImageProvi [WithFile(Issue2983, PixelTypes.Rgba32)] public void TiffDecoder_CanDecode_Issue2983(TestImageProvider provider) where TPixel : unmanaged, IPixel => TestTiffDecoder(provider); + + [Fact] + public void Identify_MalformedIccProfile_IgnoresNonCriticalErrorsByDefault() + { + ImageInfo info = Image.Identify(CreateTiffWithMalformedIccProfile()); + Assert.Equal(1, info.Width); + Assert.Equal(1, info.Height); + } + + [Fact] + public void Decode_MalformedIccProfile_IgnoresNonCriticalErrorsByDefault() + { + using Image image = Image.Load(CreateTiffWithMalformedIccProfile()); + Assert.Equal(1, image.Width); + Assert.Equal(1, image.Height); + } + + [Fact] + public void Identify_MalformedIccProfile_ThrowsWithStrict() + { + DecoderOptions options = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.Strict }; + Assert.Throws(() => Image.Identify(options, CreateTiffWithMalformedIccProfile())); + } + + [Fact] + public void Decode_MalformedIccProfile_ThrowsWithStrict() + { + DecoderOptions options = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.Strict }; + Assert.Throws(() => + { + using Image image = Image.Load(options, CreateTiffWithMalformedIccProfile()); + }); + } + + private static byte[] CreateTiffWithMalformedIccProfile() + => CorruptedMetadataImageFactory.CreateImageWithMalformedIccProfile(new TiffEncoder()); } diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs index 8d75cd9bc7..94ed54326a 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs @@ -11,6 +11,7 @@ using SixLabors.ImageSharp.Metadata.Profiles.Iptc; using SixLabors.ImageSharp.Metadata.Profiles.Xmp; using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Tests.TestDataIcc; using SixLabors.ImageSharp.Tests.TestUtilities; using static SixLabors.ImageSharp.Tests.TestImages.Tiff; @@ -335,8 +336,7 @@ public void Encode_PreservesMetadata_IptcAndIcc(TestImageProvider(TestImageProvider 0); + Assert.True(info.Height > 0); + } + + [Fact] + public void Identify_InvalidExifChunk_ThrowsWithStrict() + { + DecoderOptions options = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.Strict }; + using MemoryStream stream = new(TestFile.Create(TestImages.Webp.Lossy.WithExifNotEnoughData).Bytes, false); + Assert.Throws(() => Image.Identify(options, stream)); + } + + [Fact] + public void Decode_InvalidExifChunk_ThrowsWithStrict() + { + DecoderOptions options = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.Strict }; + using MemoryStream stream = new(TestFile.Create(TestImages.Webp.Lossy.WithExifNotEnoughData).Bytes, false); + Assert.Throws(() => + { + using Image image = Image.Load(options, stream); + }); + } } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index f7e130d66a..1a8cf948e8 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -361,6 +361,7 @@ public static class Issues public const string Issue2758 = "Jpg/issues/issue-2758.jpg"; public const string Issue2857 = "Jpg/issues/issue-2857-subsub-ifds.jpg"; public const string Issue2948 = "Jpg/issues/issue-2948-sos.jpg"; + public const string Issue3118 = "Jpg/issues/issue3118-multiple-sof.jpg"; public static class Fuzz { diff --git a/tests/ImageSharp.Tests/TestUtilities/CorruptedMetadataImageFactory.cs b/tests/ImageSharp.Tests/TestUtilities/CorruptedMetadataImageFactory.cs new file mode 100644 index 0000000000..c9d18510da --- /dev/null +++ b/tests/ImageSharp.Tests/TestUtilities/CorruptedMetadataImageFactory.cs @@ -0,0 +1,32 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using SixLabors.ImageSharp.Formats; +using SixLabors.ImageSharp.Metadata.Profiles.Icc; +using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Tests.TestDataIcc; + +namespace SixLabors.ImageSharp.Tests.TestUtilities; + +/// +/// Creates encoded images with malformed metadata payloads while keeping the surrounding container valid. +/// +internal static class CorruptedMetadataImageFactory +{ + /// + /// Creates an encoded single-pixel image with a malformed ICC profile. + /// + /// The encoder used to produce the image bytes. + /// The encoded image bytes with a malformed ICC profile payload. + public static byte[] CreateImageWithMalformedIccProfile(IImageEncoder encoder) + { + using Image image = new(1, 1); + image[0, 0] = new Rgba32(255, 0, 0, 255); + image.Metadata.IccProfile = new IccProfile(IccTestDataProfiles.HeaderInvalidSizeSmallArray); + + using MemoryStream stream = new(); + image.Save(stream, encoder); + + return stream.ToArray(); + } +} diff --git a/tests/Images/Input/Jpg/issues/issue3118-multiple-sof.jpg b/tests/Images/Input/Jpg/issues/issue3118-multiple-sof.jpg new file mode 100644 index 0000000000..e8897a511b --- /dev/null +++ b/tests/Images/Input/Jpg/issues/issue3118-multiple-sof.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:29d9073bc60cb8f5965993654ec5a949cdbacebe6365db9831ece860095ca85f +size 11541050