From af7d84a376379afd9f7214ff5f34f12f5785ff2f Mon Sep 17 00:00:00 2001 From: Scott Williams Date: Sat, 9 Apr 2022 21:06:37 +0100 Subject: [PATCH] check disposed buffers for decoders --- src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs | 9 ++- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 5 ++ .../Decompressors/JpegTiffCompression.cs | 6 +- .../Formats/Tiff/TiffDecoderCore.cs | 6 +- .../Formats/Webp/WebpDecoderCore.cs | 62 +++++++++++-------- .../Formats/Bmp/BmpDecoderTests.cs | 1 + .../Formats/Gif/GifDecoderTests.cs | 1 + .../Formats/Jpg/JpegDecoderTests.Metadata.cs | 15 +++-- .../Formats/Jpg/JpegDecoderTests.cs | 1 + .../Formats/Pbm/PbmDecoderTests.cs | 1 + .../Formats/Png/PngDecoderTests.cs | 2 +- .../Formats/Tga/TgaDecoderTests.cs | 1 + .../Formats/Tiff/TiffDecoderTests.cs | 1 + .../Formats/WebP/WebpDecoderTests.cs | 1 + .../ImageComparison/ImageComparingUtils.cs | 2 +- 15 files changed, 74 insertions(+), 40 deletions(-) diff --git a/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs b/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs index 962c9f0d67..ee0a312803 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..ab857a404a 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -151,6 +151,11 @@ namespace SixLabors.ImageSharp.Formats.Gif } } } + //catch + //{ + // image?.Dispose(); + // throw; + //} finally { this.globalColorTable?.Dispose(); diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs index cfbc32f4f6..4e788c76af 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, scanDecoderGray, 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, scanDecoder, 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..5dd99fafc1 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs @@ -240,8 +240,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 +262,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..5646f71fac 100644 --- a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs +++ b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs @@ -82,38 +82,48 @@ 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 + { - uint fileSize = this.ReadImageHeader(); + this.Metadata = new ImageMetadata(); + this.currentStream = stream; - using (this.webImageInfo = this.ReadVp8Info()) - { - if (this.webImageInfo.Features is { Animation: true }) - { - WebpThrowHelper.ThrowNotSupportedException("Animations are not supported"); - } + uint fileSize = this.ReadImageHeader(); - var image = new Image(this.Configuration, (int)this.webImageInfo.Width, (int)this.webImageInfo.Height, this.Metadata); - Buffer2D pixels = image.GetRootFramePixelBuffer(); - if (this.webImageInfo.IsLossless) + using (this.webImageInfo = this.ReadVp8Info()) { - 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); - } + if (this.webImageInfo.Features is { Animation: true }) + { + WebpThrowHelper.ThrowNotSupportedException("Animations are not supported"); + } - // There can be optional chunks after the image data, like EXIF and XMP. - if (this.webImageInfo.Features != null) - { - this.ParseOptionalChunks(this.webImageInfo.Features); - } + 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); + } - return image; + // 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; + } + } + catch + { + image?.Dispose(); + throw; } } 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.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 faf40ccf7d..1faa6f0f4c 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 { public const PixelTypes CommonNonDefaultPixelTypes = PixelTypes.Rgba32 | PixelTypes.Argb32 | PixelTypes.Bgr24 | PixelTypes.RgbaVector; 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 eadbf138a6..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; @@ -103,7 +104,6 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png [Theory] [WithFileCollection(nameof(CommonTestImages), PixelTypes.Rgba32)] - [ValidateDisposedMemoryAllocations] public void Decode(TestImageProvider provider) where TPixel : unmanaged, IPixel { 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 39fa50c93c..9460f3a351 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/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);