diff --git a/src/Directory.Build.props b/src/Directory.Build.props index d211992a94..faa29865f2 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -27,6 +27,7 @@ + diff --git a/src/ImageSharp/Diagnostics/MemoryDiagnostics.cs b/src/ImageSharp/Diagnostics/MemoryDiagnostics.cs index d83e737f12..89f18cff61 100644 --- a/src/ImageSharp/Diagnostics/MemoryDiagnostics.cs +++ b/src/ImageSharp/Diagnostics/MemoryDiagnostics.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Apache License, Version 2.0. +using System; using System.Threading; namespace SixLabors.ImageSharp.Diagnostics @@ -47,6 +48,16 @@ namespace SixLabors.ImageSharp.Diagnostics } } + /// + /// Fires when ImageSharp allocates memory from a MemoryAllocator + /// + internal static event Action MemoryAllocated; + + /// + /// Fires when ImageSharp releases memory allocated from a MemoryAllocator + /// + internal static event Action MemoryReleased; + /// /// Gets a value indicating the total number of memory resource objects leaked to the finalizer. /// @@ -54,11 +65,17 @@ namespace SixLabors.ImageSharp.Diagnostics internal static bool UndisposedAllocationSubscribed => Volatile.Read(ref undisposedAllocationSubscriptionCounter) > 0; - internal static void IncrementTotalUndisposedAllocationCount() => + internal static void IncrementTotalUndisposedAllocationCount() + { Interlocked.Increment(ref totalUndisposedAllocationCount); + MemoryAllocated?.Invoke(); + } - internal static void DecrementTotalUndisposedAllocationCount() => + internal static void DecrementTotalUndisposedAllocationCount() + { Interlocked.Decrement(ref totalUndisposedAllocationCount); + MemoryReleased?.Invoke(); + } internal static void RaiseUndisposedMemoryResource(string allocationStackTrace) { diff --git a/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs b/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs index e8cd3dcbad..a22a04980c 100644 --- a/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs +++ b/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs @@ -122,11 +122,12 @@ namespace SixLabors.ImageSharp.Formats.Bmp public Image Decode(BufferedReadStream stream, CancellationToken cancellationToken) where TPixel : unmanaged, IPixel { + Image image = null; try { int bytesPerColorMapEntry = this.ReadImageHeaders(stream, out bool inverted, out byte[] palette); - var image = new Image(this.Configuration, this.infoHeader.Width, this.infoHeader.Height, this.metadata); + image = new Image(this.Configuration, this.infoHeader.Width, this.infoHeader.Height, this.metadata); Buffer2D pixels = image.GetRootFramePixelBuffer(); @@ -193,8 +194,14 @@ namespace SixLabors.ImageSharp.Formats.Bmp } catch (IndexOutOfRangeException e) { + image?.Dispose(); throw new ImageFormatException("Bitmap does not have a valid format.", e); } + catch + { + image?.Dispose(); + throw; + } } /// 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 @@ namespace SixLabors.ImageSharp.Formats.Gif /// 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 @@ namespace SixLabors.ImageSharp.Formats.Gif /// 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 @@ namespace SixLabors.ImageSharp.Formats.Gif /// 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); } diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs index 430adeb21d..532892e060 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs @@ -95,7 +95,9 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder } } - return this.pixelBuffer; + var buffer = this.pixelBuffer; + this.pixelBuffer = null; + return buffer; } /// @@ -210,6 +212,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder this.rgbBuffer?.Dispose(); this.paddedProxyPixelRow?.Dispose(); + this.pixelBuffer?.Dispose(); } } } diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index e4a3ec1e84..6ff64a3fa5 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -237,11 +237,16 @@ namespace SixLabors.ImageSharp.Formats.Jpeg this.Metadata = new ImageMetadata(); this.QuantizationTables = new Block8x8F[4]; this.scanDecoder = scanDecoder; + 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. - stream.Read(this.markerBuffer, 0, 2); + int bytesRead = stream.Read(this.markerBuffer, 0, 2); var fileMarker = new JpegFileMarker(this.markerBuffer[1], 0); if (fileMarker.Marker != JpegConstants.Markers.SOI) { @@ -249,16 +254,23 @@ namespace SixLabors.ImageSharp.Formats.Jpeg } // Read next marker. - stream.Read(this.markerBuffer, 0, 2); - byte marker = this.markerBuffer[1]; - fileMarker = new JpegFileMarker(marker, (int)stream.Position - 2); + bytesRead = stream.Read(this.markerBuffer, 0, 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) { @@ -268,13 +280,13 @@ namespace SixLabors.ImageSharp.Formats.Jpeg 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; @@ -282,7 +294,12 @@ namespace SixLabors.ImageSharp.Formats.Jpeg } // 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); } } @@ -324,14 +341,23 @@ namespace SixLabors.ImageSharp.Formats.Jpeg if (!fileMarker.Invalid) { // Get the marker length. - int remaining = this.ReadUint16(stream) - 2; + int markerContentByteSize = this.ReadUint16(stream) - 2; + + // Check whether stream actually has enough 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) { case JpegConstants.Markers.SOF0: case JpegConstants.Markers.SOF1: case JpegConstants.Markers.SOF2: - this.ProcessStartOfFrameMarker(stream, remaining, fileMarker, ComponentType.Huffman, metadataOnly); + + this.ProcessStartOfFrameMarker(stream, markerContentByteSize, fileMarker, ComponentType.Huffman, metadataOnly); break; case JpegConstants.Markers.SOF9: @@ -344,7 +370,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg this.scanDecoder.ResetInterval = this.resetInterval.Value; } - this.ProcessStartOfFrameMarker(stream, remaining, fileMarker, ComponentType.Arithmetic, metadataOnly); + this.ProcessStartOfFrameMarker(stream, markerContentByteSize, fileMarker, ComponentType.Arithmetic, metadataOnly); break; case JpegConstants.Markers.SOF5: @@ -368,7 +394,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg case JpegConstants.Markers.SOS: if (!metadataOnly) { - this.ProcessStartOfScanMarker(stream, remaining); + this.ProcessStartOfScanMarker(stream, markerContentByteSize); break; } else @@ -382,41 +408,41 @@ namespace SixLabors.ImageSharp.Formats.Jpeg if (metadataOnly) { - stream.Skip(remaining); + stream.Skip(markerContentByteSize); } else { - 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: if (metadataOnly) { - stream.Skip(remaining); + stream.Skip(markerContentByteSize); } else { - this.ProcessDefineRestartIntervalMarker(stream, remaining); + this.ProcessDefineRestartIntervalMarker(stream, markerContentByteSize); } break; case JpegConstants.Markers.APP0: - this.ProcessApplicationHeaderMarker(stream, remaining); + this.ProcessApplicationHeaderMarker(stream, markerContentByteSize); break; case JpegConstants.Markers.APP1: - this.ProcessApp1Marker(stream, remaining); + this.ProcessApp1Marker(stream, markerContentByteSize); break; case JpegConstants.Markers.APP2: - this.ProcessApp2Marker(stream, remaining); + this.ProcessApp2Marker(stream, markerContentByteSize); break; case JpegConstants.Markers.APP3: @@ -429,30 +455,30 @@ namespace SixLabors.ImageSharp.Formats.Jpeg case JpegConstants.Markers.APP10: case JpegConstants.Markers.APP11: case JpegConstants.Markers.APP12: - stream.Skip(remaining); + stream.Skip(markerContentByteSize); break; case JpegConstants.Markers.APP13: - this.ProcessApp13Marker(stream, remaining); + this.ProcessApp13Marker(stream, markerContentByteSize); break; case JpegConstants.Markers.APP14: - this.ProcessApp14Marker(stream, remaining); + this.ProcessApp14Marker(stream, markerContentByteSize); break; case JpegConstants.Markers.APP15: case JpegConstants.Markers.COM: - stream.Skip(remaining); + stream.Skip(markerContentByteSize); break; case JpegConstants.Markers.DAC: if (metadataOnly) { - stream.Skip(remaining); + stream.Skip(markerContentByteSize); } else { - this.ProcessArithmeticTable(stream, remaining); + this.ProcessArithmeticTable(stream, markerContentByteSize); } break; @@ -748,7 +774,14 @@ namespace SixLabors.ImageSharp.Formats.Jpeg 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)) @@ -1347,7 +1380,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg int selectorsBytes = selectorsCount * 2; if (remaining != 4 + selectorsBytes) { - JpegThrowHelper.ThrowBadMarker("SOS", remaining); + JpegThrowHelper.ThrowBadMarker(nameof(JpegConstants.Markers.SOS), remaining); } // selectorsCount*2 bytes: component index + huffman tables indices @@ -1399,8 +1432,12 @@ namespace SixLabors.ImageSharp.Formats.Jpeg component.AcTableId = 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; @@ -1431,7 +1468,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg int bytesRead = stream.Read(this.markerBuffer, 0, 2); if (bytesRead != 2) { - JpegThrowHelper.ThrowInvalidImageContentException("stream does not contain enough data, could not read ushort."); + JpegThrowHelper.ThrowInvalidImageContentException("jpeg stream does not contain enough data, could not read ushort."); } return BinaryPrimitives.ReadUInt16BigEndian(this.markerBuffer); diff --git a/src/ImageSharp/Formats/Jpeg/JpegThrowHelper.cs b/src/ImageSharp/Formats/Jpeg/JpegThrowHelper.cs index b238e45ef3..1073ffff78 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegThrowHelper.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegThrowHelper.cs @@ -25,6 +25,9 @@ namespace SixLabors.ImageSharp.Formats.Jpeg [MethodImpl(InliningOptions.ColdPath)] public static void ThrowBadMarker(string marker, int length) => throw new InvalidImageContentException($"Marker {marker} has bad length {length}."); + [MethodImpl(InliningOptions.ColdPath)] + public static void ThrowNotEnoughBytesForMarker(byte marker) => throw new InvalidImageContentException($"Input stream does not have enough bytes to parse declared contents of the {marker:X2} marker."); + [MethodImpl(InliningOptions.ColdPath)] public static void ThrowBadQuantizationTableIndex(int index) => throw new InvalidImageContentException($"Bad Quantization Table index {index}."); diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 497dc39674..12770bc521 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -227,10 +227,16 @@ namespace SixLabors.ImageSharp.Formats.Png return image; } + catch + { + image?.Dispose(); + throw; + } finally { this.scanline?.Dispose(); this.previousScanline?.Dispose(); + this.nextChunk?.Data?.Dispose(); } } @@ -472,6 +478,8 @@ namespace SixLabors.ImageSharp.Formats.Png this.bytesPerSample = this.header.BitDepth / 8; } + this.previousScanline?.Dispose(); + this.scanline?.Dispose(); this.previousScanline = this.memoryAllocator.Allocate(this.bytesPerScanline, AllocationOptions.Clean); this.scanline = this.Configuration.MemoryAllocator.Allocate(this.bytesPerScanline, AllocationOptions.Clean); } @@ -1359,6 +1367,7 @@ namespace SixLabors.ImageSharp.Formats.Png { if (chunk.Type == PngChunkType.Data) { + chunk.Data?.Dispose(); return chunk.Length; } @@ -1453,6 +1462,9 @@ namespace SixLabors.ImageSharp.Formats.Png if (validCrc != inputCrc) { string chunkTypeName = Encoding.ASCII.GetString(chunkType); + + // ensure when throwing we dispose the data back to the memory allocator + chunk.Data?.Dispose(); PngThrowHelper.ThrowInvalidChunkCrc(chunkTypeName); } } diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs index ba945a8ac4..88dbcb8828 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs @@ -65,7 +65,8 @@ namespace SixLabors.ImageSharp.Formats.Tiff.Compression.Decompressors jpegDecoder.ParseStream(stream, spectralConverterGray, CancellationToken.None); // TODO: Should we pass through the CancellationToken from the tiff decoder? - CopyImageBytesToBuffer(buffer, spectralConverterGray.GetPixelBuffer(CancellationToken.None)); + using var decompressedBuffer = spectralConverterGray.GetPixelBuffer(CancellationToken.None); + CopyImageBytesToBuffer(buffer, decompressedBuffer); break; } @@ -81,7 +82,8 @@ namespace SixLabors.ImageSharp.Formats.Tiff.Compression.Decompressors jpegDecoder.ParseStream(stream, spectralConverter, CancellationToken.None); // TODO: Should we pass through the CancellationToken from the tiff decoder? - CopyImageBytesToBuffer(buffer, spectralConverter.GetPixelBuffer(CancellationToken.None)); + using var decompressedBuffer = spectralConverter.GetPixelBuffer(CancellationToken.None); + CopyImageBytesToBuffer(buffer, decompressedBuffer); break; } diff --git a/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs b/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs index 1198c519a2..1cd3d2c0c1 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs @@ -157,40 +157,52 @@ namespace SixLabors.ImageSharp.Formats.Tiff public Image Decode(BufferedReadStream stream, CancellationToken cancellationToken) where TPixel : unmanaged, IPixel { - this.inputStream = stream; - var reader = new DirectoryReader(stream, this.Configuration.MemoryAllocator); - - IEnumerable directories = reader.Read(); - this.byteOrder = reader.ByteOrder; - this.isBigTiff = reader.IsBigTiff; - var frames = new List>(); - foreach (ExifProfile ifd in directories) + try { - cancellationToken.ThrowIfCancellationRequested(); - ImageFrame frame = this.DecodeFrame(ifd, cancellationToken); - frames.Add(frame); + this.inputStream = stream; + var reader = new DirectoryReader(stream, this.Configuration.MemoryAllocator); + + IEnumerable directories = reader.Read(); + this.byteOrder = reader.ByteOrder; + this.isBigTiff = reader.IsBigTiff; - if (this.decodingMode is FrameDecodingMode.First) + foreach (ExifProfile ifd in directories) { - break; + cancellationToken.ThrowIfCancellationRequested(); + ImageFrame frame = this.DecodeFrame(ifd, cancellationToken); + frames.Add(frame); + + if (this.decodingMode is FrameDecodingMode.First) + { + break; + } } - } - ImageMetadata metadata = TiffDecoderMetadataCreator.Create(frames, this.ignoreMetadata, reader.ByteOrder, reader.IsBigTiff); + ImageMetadata metadata = TiffDecoderMetadataCreator.Create(frames, this.ignoreMetadata, reader.ByteOrder, reader.IsBigTiff); - // TODO: Tiff frames can have different sizes. - ImageFrame root = frames[0]; - this.Dimensions = root.Size(); - foreach (ImageFrame frame in frames) - { - if (frame.Size() != root.Size()) + // TODO: Tiff frames can have different sizes. + ImageFrame root = frames[0]; + this.Dimensions = root.Size(); + foreach (ImageFrame frame in frames) { - TiffThrowHelper.ThrowNotSupported("Images with different sizes are not supported"); + if (frame.Size() != root.Size()) + { + TiffThrowHelper.ThrowNotSupported("Images with different sizes are not supported"); + } } + + return new Image(this.Configuration, metadata, frames); } + catch + { + foreach (ImageFrame f in frames) + { + f.Dispose(); + } - return new Image(this.Configuration, metadata, frames); + throw; + } } /// @@ -240,8 +252,8 @@ namespace SixLabors.ImageSharp.Formats.Tiff var stripOffsetsArray = (Array)tags.GetValueInternal(ExifTag.StripOffsets).GetValue(); var stripByteCountsArray = (Array)tags.GetValueInternal(ExifTag.StripByteCounts).GetValue(); - IMemoryOwner stripOffsetsMemory = this.ConvertNumbers(stripOffsetsArray, out Span stripOffsets); - IMemoryOwner stripByteCountsMemory = this.ConvertNumbers(stripByteCountsArray, out Span stripByteCounts); + using IMemoryOwner stripOffsetsMemory = this.ConvertNumbers(stripOffsetsArray, out Span stripOffsets); + using IMemoryOwner stripByteCountsMemory = this.ConvertNumbers(stripByteCountsArray, out Span stripByteCounts); if (this.PlanarConfiguration == TiffPlanarConfiguration.Planar) { @@ -262,8 +274,6 @@ namespace SixLabors.ImageSharp.Formats.Tiff cancellationToken); } - stripOffsetsMemory?.Dispose(); - stripByteCountsMemory?.Dispose(); return frame; } diff --git a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs index 9d18e5d821..0e00f037ca 100644 --- a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs +++ b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs @@ -82,38 +82,47 @@ namespace SixLabors.ImageSharp.Formats.Webp public Image Decode(BufferedReadStream stream, CancellationToken cancellationToken) where TPixel : unmanaged, IPixel { - this.Metadata = new ImageMetadata(); - this.currentStream = stream; + Image image = null; + try + { + this.Metadata = new ImageMetadata(); + this.currentStream = stream; - uint fileSize = this.ReadImageHeader(); + uint fileSize = this.ReadImageHeader(); - using (this.webImageInfo = this.ReadVp8Info()) - { - if (this.webImageInfo.Features is { Animation: true }) + using (this.webImageInfo = this.ReadVp8Info()) { - WebpThrowHelper.ThrowNotSupportedException("Animations are not supported"); - } + if (this.webImageInfo.Features is { Animation: true }) + { + WebpThrowHelper.ThrowNotSupportedException("Animations are not supported"); + } - var image = new Image(this.Configuration, (int)this.webImageInfo.Width, (int)this.webImageInfo.Height, this.Metadata); - Buffer2D pixels = image.GetRootFramePixelBuffer(); - if (this.webImageInfo.IsLossless) - { - var losslessDecoder = new WebpLosslessDecoder(this.webImageInfo.Vp8LBitReader, this.memoryAllocator, this.Configuration); - losslessDecoder.Decode(pixels, image.Width, image.Height); - } - else - { - var lossyDecoder = new WebpLossyDecoder(this.webImageInfo.Vp8BitReader, this.memoryAllocator, this.Configuration); - lossyDecoder.Decode(pixels, image.Width, image.Height, this.webImageInfo); - } + image = new Image(this.Configuration, (int)this.webImageInfo.Width, (int)this.webImageInfo.Height, this.Metadata); + Buffer2D pixels = image.GetRootFramePixelBuffer(); + if (this.webImageInfo.IsLossless) + { + var losslessDecoder = new WebpLosslessDecoder(this.webImageInfo.Vp8LBitReader, this.memoryAllocator, this.Configuration); + losslessDecoder.Decode(pixels, image.Width, image.Height); + } + else + { + var lossyDecoder = new WebpLossyDecoder(this.webImageInfo.Vp8BitReader, this.memoryAllocator, this.Configuration); + lossyDecoder.Decode(pixels, image.Width, image.Height, this.webImageInfo); + } - // There can be optional chunks after the image data, like EXIF and XMP. - if (this.webImageInfo.Features != null) - { - this.ParseOptionalChunks(this.webImageInfo.Features); - } + // There can be optional chunks after the image data, like EXIF and XMP. + if (this.webImageInfo.Features != null) + { + this.ParseOptionalChunks(this.webImageInfo.Features); + } - return image; + return image; + } + } + catch + { + image?.Dispose(); + throw; } } @@ -190,7 +199,11 @@ namespace SixLabors.ImageSharp.Formats.Webp 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 +227,34 @@ namespace SixLabors.ImageSharp.Formats.Webp 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; @@ -272,7 +300,12 @@ namespace SixLabors.ImageSharp.Formats.Webp 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 +317,12 @@ namespace SixLabors.ImageSharp.Formats.Webp // - 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 +350,23 @@ namespace SixLabors.ImageSharp.Formats.Webp } // 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); @@ -429,54 +477,15 @@ namespace SixLabors.ImageSharp.Formats.Webp switch (chunkType) { case WebpChunkType.Iccp: - uint iccpChunkSize = this.ReadChunkSize(); - if (this.IgnoreMetadata) - { - this.currentStream.Skip((int)iccpChunkSize); - } - else - { - byte[] iccpData = new byte[iccpChunkSize]; - this.currentStream.Read(iccpData, 0, (int)iccpChunkSize); - 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]; - this.currentStream.Read(exifData, 0, (int)exifChunkSize); - 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]; - this.currentStream.Read(xmpData, 0, (int)xmpChunkSize); - var profile = new XmpProfile(xmpData); - this.Metadata.XmpProfile = profile; - } - + this.ReadXmpProfile(); break; case WebpChunkType.Animation: @@ -488,7 +497,12 @@ namespace SixLabors.ImageSharp.Formats.Webp 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); + int 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"); @@ -514,22 +528,100 @@ namespace SixLabors.ImageSharp.Formats.Webp { // 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) + { + // Ignore invalid chunk. + return; + } + + 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) + { + // Ignore invalid chunk. + return; + } + + 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. /// 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 /// diff --git a/src/ImageSharp/IO/BufferedReadStream.cs b/src/ImageSharp/IO/BufferedReadStream.cs index 4ab7f312b2..2823b8ed6f 100644 --- a/src/ImageSharp/IO/BufferedReadStream.cs +++ b/src/ImageSharp/IO/BufferedReadStream.cs @@ -114,6 +114,15 @@ namespace SixLabors.ImageSharp.IO /// public override bool CanWrite { get; } = false; + /// + /// Gets remaining byte count available to read. + /// + public long RemainingBytes + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => this.Length - this.Position; + } + /// /// Gets the underlying stream. /// diff --git a/src/ImageSharp/Metadata/Profiles/Exif/ExifEncodedStringHelpers.cs b/src/ImageSharp/Metadata/Profiles/Exif/ExifEncodedStringHelpers.cs index 5fd613b1f0..4ec9b3267e 100644 --- a/src/ImageSharp/Metadata/Profiles/Exif/ExifEncodedStringHelpers.cs +++ b/src/ImageSharp/Metadata/Profiles/Exif/ExifEncodedStringHelpers.cs @@ -80,7 +80,15 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Exif } public static unsafe int Write(Encoding encoding, string value, Span destination) +#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP2_1_OR_GREATER || NET + => encoding.GetBytes(value.AsSpan(), destination); +#else { + if (value.Length == 0) + { + return 0; + } + fixed (char* c = value) { fixed (byte* b = destination) @@ -89,6 +97,7 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Exif } } } +#endif private static bool TryDetect(ReadOnlySpan buffer, out CharacterCode code) { diff --git a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs index 71d0ab34a4..43ec45a34f 100644 --- a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs @@ -20,6 +20,7 @@ using static SixLabors.ImageSharp.Tests.TestImages.Bmp; namespace SixLabors.ImageSharp.Tests.Formats.Bmp { [Trait("Format", "Bmp")] + [ValidateDisposedMemoryAllocations] public class BmpDecoderTests { public const PixelTypes CommonNonDefaultPixelTypes = PixelTypes.Rgba32 | PixelTypes.Bgra32 | PixelTypes.RgbaVector; diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index 1922b161f2..7a5241c5a8 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -18,6 +18,7 @@ using Xunit; namespace SixLabors.ImageSharp.Tests.Formats.Gif { [Trait("Format", "Gif")] + [ValidateDisposedMemoryAllocations] public class GifDecoderTests { private const PixelTypes TestPixelTypes = PixelTypes.Rgba32 | PixelTypes.RgbaVector | PixelTypes.Argb32; diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs index 8db3f062fd..543619c876 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs @@ -101,6 +101,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg TestImages.Jpeg.Issues.Fuzz.IndexOutOfRangeException1693A, TestImages.Jpeg.Issues.Fuzz.IndexOutOfRangeException1693B, TestImages.Jpeg.Issues.Fuzz.IndexOutOfRangeException824C, + TestImages.Jpeg.Issues.Fuzz.NullReferenceException2085, }; private static readonly Dictionary CustomToleranceValues = new() diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs index 9864d62b8f..d9915f17d6 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs @@ -179,11 +179,16 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg var testFile = TestFile.Create(imagePath); using (var stream = new MemoryStream(testFile.Bytes, false)) { - IImageInfo imageInfo = useIdentify - ? ((IImageInfoDetector)decoder).Identify(Configuration.Default, stream, default) - : decoder.Decode(Configuration.Default, stream, default); - - test(imageInfo); + if (useIdentify) + { + IImageInfo imageInfo = ((IImageInfoDetector)decoder).Identify(Configuration.Default, stream, default); + test(imageInfo); + } + else + { + using var img = decoder.Decode(Configuration.Default, stream, default); + test(img); + } } } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index 497479096c..e39aaa323e 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -22,6 +22,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg { // TODO: Scatter test cases into multiple test classes [Trait("Format", "Jpg")] + [ValidateDisposedMemoryAllocations] public partial class JpegDecoderTests { private static MagickReferenceDecoder ReferenceDecoder => new(); diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs new file mode 100644 index 0000000000..2bbce6cb1b --- /dev/null +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs @@ -0,0 +1,33 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +using System; +using System.IO; +using SixLabors.ImageSharp.Formats.Jpeg; +using SixLabors.ImageSharp.PixelFormats; + +using Xunit; + +namespace SixLabors.ImageSharp.Tests.Formats.Jpg +{ + [Trait("Format", "Jpg")] + public partial class JpegEncoderTests + { + [Theory] + [WithFile(TestImages.Jpeg.Issues.ValidExifArgumentNullExceptionOnEncode, PixelTypes.Rgba32)] + public void Encode_WithValidExifProfile_DoesNotThrowException(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + Exception ex = Record.Exception(() => + { + var encoder = new JpegEncoder(); + var stream = new MemoryStream(); + + using Image image = provider.GetImage(JpegDecoder); + image.Save(stream, encoder); + }); + + Assert.Null(ex); + } + } +} diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.cs index 18eae9fbd3..d860836e08 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.cs @@ -20,7 +20,7 @@ using Xunit; namespace SixLabors.ImageSharp.Tests.Formats.Jpg { [Trait("Format", "Jpg")] - public class JpegEncoderTests + public partial class JpegEncoderTests { private static JpegEncoder JpegEncoder => new(); diff --git a/tests/ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs index 97237bca59..eb3bc8c9a5 100644 --- a/tests/ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs @@ -11,6 +11,7 @@ using static SixLabors.ImageSharp.Tests.TestImages.Pbm; namespace SixLabors.ImageSharp.Tests.Formats.Pbm { [Trait("Format", "Pbm")] + [ValidateDisposedMemoryAllocations] public class PbmDecoderTests { [Theory] diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 752036126f..a4fcf63baf 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -19,6 +19,7 @@ using Xunit; namespace SixLabors.ImageSharp.Tests.Formats.Png { [Trait("Format", "Png")] + [ValidateDisposedMemoryAllocations] public partial class PngDecoderTests { private const PixelTypes TestPixelTypes = PixelTypes.Rgba32 | PixelTypes.RgbaVector | PixelTypes.Argb32; diff --git a/tests/ImageSharp.Tests/Formats/Tga/TgaDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Tga/TgaDecoderTests.cs index 55dc2ecdd8..e83c5a98cc 100644 --- a/tests/ImageSharp.Tests/Formats/Tga/TgaDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tga/TgaDecoderTests.cs @@ -16,6 +16,7 @@ using static SixLabors.ImageSharp.Tests.TestImages.Tga; namespace SixLabors.ImageSharp.Tests.Formats.Tga { [Trait("Format", "Tga")] + [ValidateDisposedMemoryAllocations] public class TgaDecoderTests { private static TgaDecoder TgaDecoder => new(); diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs index 914d80a58d..ceded79cc2 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs @@ -14,6 +14,7 @@ using static SixLabors.ImageSharp.Tests.TestImages.Tiff; namespace SixLabors.ImageSharp.Tests.Formats.Tiff { [Trait("Format", "Tiff")] + [ValidateDisposedMemoryAllocations] public class TiffDecoderTests : TiffDecoderBaseTester { public static readonly string[] MultiframeTestImages = Multiframes; diff --git a/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs b/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs index 1c92fdf335..f29fa5d793 100644 --- a/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs @@ -13,6 +13,7 @@ using static SixLabors.ImageSharp.Tests.TestImages.Webp; namespace SixLabors.ImageSharp.Tests.Formats.Webp { [Trait("Format", "Webp")] + [ValidateDisposedMemoryAllocations] public class WebpDecoderTests { private static WebpDecoder WebpDecoder => new(); diff --git a/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs b/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs index eaa7fb5646..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; @@ -150,5 +151,17 @@ namespace SixLabors.ImageSharp.Tests.Formats.Webp Assert.NotNull(actualExif); Assert.Equal(expectedExif.Values.Count, actualExif.Values.Count); } + + [Theory] + [WithFile(TestImages.Webp.Lossy.WithExifNotEnoughData, PixelTypes.Rgba32)] + public void WebpDecoder_IgnoresInvalidExifChunk(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + Exception ex = Record.Exception(() => + { + using Image image = provider.GetImage(); + }); + Assert.Null(ex); + } } } diff --git a/tests/ImageSharp.Tests/Image/ImageTests.LoadPixelData.cs b/tests/ImageSharp.Tests/Image/ImageTests.LoadPixelData.cs index a4044b906c..7683ee6889 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.LoadPixelData.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.LoadPixelData.cs @@ -14,6 +14,7 @@ namespace SixLabors.ImageSharp.Tests [Theory] [InlineData(false)] [InlineData(true)] + [ValidateDisposedMemoryAllocations] public void FromPixels(bool useSpan) { Rgba32[] data = { Color.Black, Color.White, Color.White, Color.Black, }; diff --git a/tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs b/tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs index 357d02be4b..ce1f902e59 100644 --- a/tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs +++ b/tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs @@ -55,6 +55,8 @@ namespace SixLabors.ImageSharp.Tests static void RunTest(string formatInner) { + using IDisposable mem = MemoryAllocatorValidator.MonitorAllocations(); + Configuration configuration = Configuration.Default.Clone(); configuration.PreferContiguousImageBuffers = true; IImageEncoder encoder = configuration.ImageFormatsManager.FindEncoder( diff --git a/tests/ImageSharp.Tests/MemoryAllocatorValidator.cs b/tests/ImageSharp.Tests/MemoryAllocatorValidator.cs new file mode 100644 index 0000000000..13664ee9b2 --- /dev/null +++ b/tests/ImageSharp.Tests/MemoryAllocatorValidator.cs @@ -0,0 +1,77 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +using System; +using System.Threading; +using SixLabors.ImageSharp.Diagnostics; +using Xunit; + +namespace SixLabors.ImageSharp.Tests +{ + public static class MemoryAllocatorValidator + { + private static readonly AsyncLocal LocalInstance = new(); + + public static bool MonitoringAllocations => LocalInstance.Value != null; + + static MemoryAllocatorValidator() + { + MemoryDiagnostics.MemoryAllocated += MemoryDiagnostics_MemoryAllocated; + MemoryDiagnostics.MemoryReleased += MemoryDiagnostics_MemoryReleased; + } + + private static void MemoryDiagnostics_MemoryReleased() + { + TestMemoryDiagnostics backing = LocalInstance.Value; + if (backing != null) + { + backing.TotalRemainingAllocated--; + } + } + + private static void MemoryDiagnostics_MemoryAllocated() + { + TestMemoryDiagnostics backing = LocalInstance.Value; + if (backing != null) + { + backing.TotalAllocated++; + backing.TotalRemainingAllocated++; + } + } + + public static TestMemoryDiagnostics MonitorAllocations() + { + var diag = new TestMemoryDiagnostics(); + LocalInstance.Value = diag; + return diag; + } + + public static void StopMonitoringAllocations() => LocalInstance.Value = null; + + public static void ValidateAllocations(int expectedAllocationCount = 0) + => LocalInstance.Value?.Validate(expectedAllocationCount); + + public class TestMemoryDiagnostics : IDisposable + { + public int TotalAllocated { get; set; } + + public int TotalRemainingAllocated { get; set; } + + public void Validate(int expectedAllocationCount) + { + var count = this.TotalRemainingAllocated; + var pass = expectedAllocationCount == count; + Assert.True(pass, $"Expected a {expectedAllocationCount} undisposed buffers but found {count}"); + } + + public void Dispose() + { + this.Validate(0); + if (LocalInstance.Value == this) + { + StopMonitoringAllocations(); + } + } + } + } +} diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 060ed3f32e..fa51fb2254 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -271,6 +271,7 @@ namespace SixLabors.ImageSharp.Tests public const string InvalidIptcTag = "Jpg/issues/Issue1942InvalidIptcTag.jpg"; public const string Issue2057App1Parsing = "Jpg/issues/Issue2057-App1Parsing.jpg"; public const string ExifNullArrayTag = "Jpg/issues/issue-2056-exif-null-array.jpg"; + public const string ValidExifArgumentNullExceptionOnEncode = "Jpg/issues/Issue2087-exif-null-reference-on-encode.jpg"; public static class Fuzz { @@ -299,6 +300,7 @@ namespace SixLabors.ImageSharp.Tests public const string AccessViolationException922 = "Jpg/issues/fuzz/Issue922-AccessViolationException.jpg"; public const string IndexOutOfRangeException1693A = "Jpg/issues/fuzz/Issue1693-IndexOutOfRangeException-A.jpg"; public const string IndexOutOfRangeException1693B = "Jpg/issues/fuzz/Issue1693-IndexOutOfRangeException-B.jpg"; + public const string NullReferenceException2085 = "Jpg/issues/fuzz/Issue2085-NullReferenceException.jpg"; } } @@ -642,6 +644,7 @@ namespace SixLabors.ImageSharp.Tests { 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/ImageSharp.Tests/TestUtilities/ImageComparison/ImageComparingUtils.cs b/tests/ImageSharp.Tests/TestUtilities/ImageComparison/ImageComparingUtils.cs index 1801d6b590..fa25846748 100644 --- a/tests/ImageSharp.Tests/TestUtilities/ImageComparison/ImageComparingUtils.cs +++ b/tests/ImageSharp.Tests/TestUtilities/ImageComparison/ImageComparingUtils.cs @@ -26,7 +26,7 @@ namespace SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison } var testFile = TestFile.Create(path); - Image magickImage = DecodeWithMagick(new FileInfo(testFile.FullPath)); + using Image magickImage = DecodeWithMagick(new FileInfo(testFile.FullPath)); if (useExactComparer) { ImageComparer.Exact.VerifySimilarity(magickImage, image); diff --git a/tests/ImageSharp.Tests/TestUtilities/ImageProviders/FileProvider.cs b/tests/ImageSharp.Tests/TestUtilities/ImageProviders/FileProvider.cs index e2e7d73bc6..63c5ce31a2 100644 --- a/tests/ImageSharp.Tests/TestUtilities/ImageProviders/FileProvider.cs +++ b/tests/ImageSharp.Tests/TestUtilities/ImageProviders/FileProvider.cs @@ -7,6 +7,7 @@ using System.Collections.Generic; using System.IO; using System.Reflection; using System.Threading.Tasks; +using SixLabors.ImageSharp.Diagnostics; using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.PixelFormats; @@ -158,8 +159,13 @@ namespace SixLabors.ImageSharp.Tests return this.LoadImage(decoder); } - var key = new Key(this.PixelType, this.FilePath, decoder); + // do not cache so we can track allocation correctly when validating memory + if (MemoryAllocatorValidator.MonitoringAllocations) + { + return this.LoadImage(decoder); + } + var key = new Key(this.PixelType, this.FilePath, decoder); Image cachedImage = Cache.GetOrAdd(key, _ => this.LoadImage(decoder)); return cachedImage.Clone(this.Configuration); diff --git a/tests/ImageSharp.Tests/ValidateDisposedMemoryAllocationsAttribute.cs b/tests/ImageSharp.Tests/ValidateDisposedMemoryAllocationsAttribute.cs new file mode 100644 index 0000000000..65ed990dd7 --- /dev/null +++ b/tests/ImageSharp.Tests/ValidateDisposedMemoryAllocationsAttribute.cs @@ -0,0 +1,33 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +using System; +using System.Diagnostics; +using System.Reflection; +using Xunit.Sdk; + +namespace SixLabors.ImageSharp.Tests +{ + [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)] + public class ValidateDisposedMemoryAllocationsAttribute : BeforeAfterTestAttribute + { + private readonly int expected = 0; + + public ValidateDisposedMemoryAllocationsAttribute() + : this(0) + { + } + + public ValidateDisposedMemoryAllocationsAttribute(int expected) + => this.expected = expected; + + public override void Before(MethodInfo methodUnderTest) + => MemoryAllocatorValidator.MonitorAllocations(); + + public override void After(MethodInfo methodUnderTest) + { + MemoryAllocatorValidator.ValidateAllocations(this.expected); + MemoryAllocatorValidator.StopMonitoringAllocations(); + } + } +} diff --git a/tests/Images/Input/Jpg/issues/Issue2087-exif-null-reference-on-encode.jpg b/tests/Images/Input/Jpg/issues/Issue2087-exif-null-reference-on-encode.jpg new file mode 100644 index 0000000000..e95ef7a73d --- /dev/null +++ b/tests/Images/Input/Jpg/issues/Issue2087-exif-null-reference-on-encode.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:4d41a41180a3371d0c4a724b40a4c86f6f975dab6be9da96964a484818770394 +size 30715 diff --git a/tests/Images/Input/Jpg/issues/fuzz/Issue2085-NullReferenceException.jpg b/tests/Images/Input/Jpg/issues/fuzz/Issue2085-NullReferenceException.jpg new file mode 100644 index 0000000000..8a680ff6a6 --- /dev/null +++ b/tests/Images/Input/Jpg/issues/fuzz/Issue2085-NullReferenceException.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:d478beff34179fda26238a44434607c276f55438ee96824c5af8c0188d358d8d +size 234 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 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