diff --git a/.gitattributes b/.gitattributes index 355b64dce1..f97695f901 100644 --- a/.gitattributes +++ b/.gitattributes @@ -115,6 +115,7 @@ *.jpg filter=lfs diff=lfs merge=lfs -text *.jpeg filter=lfs diff=lfs merge=lfs -text *.bmp filter=lfs diff=lfs merge=lfs -text +*.BMP filter=lfs diff=lfs merge=lfs -text *.gif filter=lfs diff=lfs merge=lfs -text *.png filter=lfs diff=lfs merge=lfs -text *.tif filter=lfs diff=lfs merge=lfs -text diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 5b3d294bc5..7ccf2fc8ec 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -74,6 +74,16 @@ jobs: runs-on: ${{matrix.options.os}} steps: + - name: Install Ubuntu prerequisites + if: ${{ contains(matrix.options.os, 'ubuntu') }} + run: | + # libssl 1.1 (required by old .NET runtimes) + wget http://archive.ubuntu.com/ubuntu/pool/main/o/openssl/libssl1.1_1.1.0g-2ubuntu4_amd64.deb + sudo dpkg -i libssl1.1_1.1.0g-2ubuntu4_amd64.deb + + # libgdiplus + sudo apt-get -y install libgdiplus libgif-dev libglib2.0-dev libcairo2-dev libtiff-dev libexif-dev + - name: Git Config shell: bash run: | @@ -151,7 +161,7 @@ jobs: XUNIT_PATH: .\tests\ImageSharp.Tests # Required for xunit - name: Export Failed Output - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 if: failure() with: name: actual_output_${{ runner.os }}_${{ matrix.options.framework }}${{ matrix.options.runtime }}.zip diff --git a/src/ImageSharp/Formats/Gif/LzwDecoder.cs b/src/ImageSharp/Formats/Gif/LzwDecoder.cs index aa4327a1a0..6ad88f95cf 100644 --- a/src/ImageSharp/Formats/Gif/LzwDecoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwDecoder.cs @@ -38,22 +38,22 @@ internal sealed class LzwDecoder : IDisposable /// /// The prefix buffer. /// - private readonly IMemoryOwner prefix; + private readonly IMemoryOwner prefixOwner; /// /// The suffix buffer. /// - private readonly IMemoryOwner suffix; + private readonly IMemoryOwner suffixOwner; /// /// The scratch buffer for reading data blocks. /// - private readonly IMemoryOwner scratchBuffer; + private readonly IMemoryOwner bufferOwner; /// /// The pixel stack buffer. /// - private readonly IMemoryOwner pixelStack; + private readonly IMemoryOwner pixelStackOwner; private readonly int minCodeSize; private readonly int clearCode; private readonly int endCode; @@ -80,11 +80,12 @@ internal sealed class LzwDecoder : IDisposable public LzwDecoder(MemoryAllocator memoryAllocator, BufferedReadStream stream, int minCodeSize) { this.stream = stream ?? throw new ArgumentNullException(nameof(stream)); + Guard.IsTrue(IsValidMinCodeSize(minCodeSize), nameof(minCodeSize), "Invalid minimum code size."); - this.prefix = memoryAllocator.Allocate(MaxStackSize, AllocationOptions.Clean); - this.suffix = memoryAllocator.Allocate(MaxStackSize, AllocationOptions.Clean); - this.pixelStack = memoryAllocator.Allocate(MaxStackSize + 1, AllocationOptions.Clean); - this.scratchBuffer = memoryAllocator.Allocate(byte.MaxValue, AllocationOptions.None); + this.prefixOwner = memoryAllocator.Allocate(MaxStackSize, AllocationOptions.Clean); + this.suffixOwner = memoryAllocator.Allocate(MaxStackSize, AllocationOptions.Clean); + this.pixelStackOwner = memoryAllocator.Allocate(MaxStackSize + 1, AllocationOptions.Clean); + this.bufferOwner = memoryAllocator.Allocate(byte.MaxValue, AllocationOptions.None); this.minCodeSize = minCodeSize; // Calculate the clear code. The value of the clear code is 2 ^ minCodeSize @@ -94,11 +95,15 @@ public LzwDecoder(MemoryAllocator memoryAllocator, BufferedReadStream stream, in this.endCode = this.clearCode + 1; this.availableCode = this.clearCode + 2; - ref int suffixRef = ref MemoryMarshal.GetReference(this.suffix.GetSpan()); - for (this.code = 0; this.code < this.clearCode; this.code++) + // Fill the suffix buffer with the initial values represented by the number of colors. + Span suffix = this.suffixOwner.GetSpan().Slice(0, this.clearCode); + int i; + for (i = 0; i < suffix.Length; i++) { - Unsafe.Add(ref suffixRef, this.code) = (byte)this.code; + suffix[i] = i; } + + this.code = i; } /// @@ -113,8 +118,7 @@ public static bool IsValidMinCodeSize(int minCodeSize) // It is possible to specify a larger LZW minimum code size than the palette length in bits // which may leave a gap in the codes where no colors are assigned. // http://www.matthewflickinger.com/lab/whatsinagif/lzw_image_data.asp#lzw_compression - int clearCode = 1 << minCodeSize; - if (minCodeSize < 2 || minCodeSize > MaximumLzwBits || clearCode > MaxStackSize) + if (minCodeSize < 2 || minCodeSize > MaximumLzwBits || 1 << minCodeSize > MaxStackSize) { // Don't attempt to decode the frame indices. // Theoretically we could determine a min code size from the length of the provided @@ -133,112 +137,139 @@ public void DecodePixelRow(Span indices) { indices.Clear(); - ref byte pixelsRowRef = ref MemoryMarshal.GetReference(indices); - ref int prefixRef = ref MemoryMarshal.GetReference(this.prefix.GetSpan()); - ref int suffixRef = ref MemoryMarshal.GetReference(this.suffix.GetSpan()); - ref int pixelStackRef = ref MemoryMarshal.GetReference(this.pixelStack.GetSpan()); - Span buffer = this.scratchBuffer.GetSpan(); - - int x = 0; - int xyz = 0; - while (xyz < indices.Length) + // Get span values from the owners. + Span prefix = this.prefixOwner.GetSpan(); + Span suffix = this.suffixOwner.GetSpan(); + Span pixelStack = this.pixelStackOwner.GetSpan(); + Span buffer = this.bufferOwner.GetSpan(); + + // Cache frequently accessed instance fields into locals. + // This helps avoid repeated field loads inside the tight loop. + BufferedReadStream stream = this.stream; + int top = this.top; + int bits = this.bits; + int codeSize = this.codeSize; + int codeMask = this.codeMask; + int minCodeSize = this.minCodeSize; + int availableCode = this.availableCode; + int oldCode = this.oldCode; + int first = this.first; + int data = this.data; + int count = this.count; + int bufferIndex = this.bufferIndex; + int code = this.code; + int clearCode = this.clearCode; + int endCode = this.endCode; + + int i = 0; + while (i < indices.Length) { - if (this.top == 0) + if (top == 0) { - if (this.bits < this.codeSize) + if (bits < codeSize) { // Load bytes until there are enough bits for a code. - if (this.count == 0) + if (count == 0) { // Read a new data block. - this.count = this.ReadBlock(buffer); - if (this.count == 0) + count = ReadBlock(stream, buffer); + if (count == 0) { break; } - this.bufferIndex = 0; + bufferIndex = 0; } - this.data += buffer[this.bufferIndex] << this.bits; - - this.bits += 8; - this.bufferIndex++; - this.count--; + data += buffer[bufferIndex] << bits; + bits += 8; + bufferIndex++; + count--; continue; } // Get the next code - this.code = this.data & this.codeMask; - this.data >>= this.codeSize; - this.bits -= this.codeSize; + code = data & codeMask; + data >>= codeSize; + bits -= codeSize; // Interpret the code - if (this.code > this.availableCode || this.code == this.endCode) + if (code > availableCode || code == endCode) { break; } - if (this.code == this.clearCode) + if (code == clearCode) { // Reset the decoder - this.codeSize = this.minCodeSize + 1; - this.codeMask = (1 << this.codeSize) - 1; - this.availableCode = this.clearCode + 2; - this.oldCode = NullCode; + codeSize = minCodeSize + 1; + codeMask = (1 << codeSize) - 1; + availableCode = clearCode + 2; + oldCode = NullCode; continue; } - if (this.oldCode == NullCode) + if (oldCode == NullCode) { - Unsafe.Add(ref pixelStackRef, this.top++) = Unsafe.Add(ref suffixRef, this.code); - this.oldCode = this.code; - this.first = this.code; + pixelStack[top++] = suffix[code]; + oldCode = code; + first = code; continue; } - int inCode = this.code; - if (this.code == this.availableCode) + int inCode = code; + if (code == availableCode) { - Unsafe.Add(ref pixelStackRef, this.top++) = (byte)this.first; - - this.code = this.oldCode; + pixelStack[top++] = first; + code = oldCode; } - while (this.code > this.clearCode) + while (code > clearCode && top < MaxStackSize) { - Unsafe.Add(ref pixelStackRef, this.top++) = Unsafe.Add(ref suffixRef, this.code); - this.code = Unsafe.Add(ref prefixRef, this.code); + pixelStack[top++] = suffix[code]; + code = prefix[code]; } - int suffixCode = Unsafe.Add(ref suffixRef, this.code); - this.first = suffixCode; - Unsafe.Add(ref pixelStackRef, this.top++) = suffixCode; + int suffixCode = suffix[code]; + first = suffixCode; + pixelStack[top++] = suffixCode; - // Fix for Gifs that have "deferred clear code" as per here : + // Fix for GIFs that have "deferred clear code" as per: // https://bugzilla.mozilla.org/show_bug.cgi?id=55918 - if (this.availableCode < MaxStackSize) + if (availableCode < MaxStackSize) { - Unsafe.Add(ref prefixRef, this.availableCode) = this.oldCode; - Unsafe.Add(ref suffixRef, this.availableCode) = this.first; - this.availableCode++; - if (this.availableCode == this.codeMask + 1 && this.availableCode < MaxStackSize) + prefix[availableCode] = oldCode; + suffix[availableCode] = first; + availableCode++; + if (availableCode == codeMask + 1 && availableCode < MaxStackSize) { - this.codeSize++; - this.codeMask = (1 << this.codeSize) - 1; + codeSize++; + codeMask = (1 << codeSize) - 1; } } - this.oldCode = inCode; + oldCode = inCode; } // Pop a pixel off the pixel stack. - this.top--; + top--; - // Clear missing pixels - xyz++; - Unsafe.Add(ref pixelsRowRef, x++) = (byte)Unsafe.Add(ref pixelStackRef, this.top); + // Clear missing pixels. + indices[i++] = (byte)pixelStack[top]; } + + // Write back the local values to the instance fields. + this.top = top; + this.bits = bits; + this.codeSize = codeSize; + this.codeMask = codeMask; + this.availableCode = availableCode; + this.oldCode = oldCode; + this.first = first; + this.data = data; + this.count = count; + this.bufferIndex = bufferIndex; + this.code = code; } /// @@ -247,130 +278,161 @@ public void DecodePixelRow(Span indices) /// The resulting index table length. public void SkipIndices(int length) { - ref int prefixRef = ref MemoryMarshal.GetReference(this.prefix.GetSpan()); - ref int suffixRef = ref MemoryMarshal.GetReference(this.suffix.GetSpan()); - ref int pixelStackRef = ref MemoryMarshal.GetReference(this.pixelStack.GetSpan()); - Span buffer = this.scratchBuffer.GetSpan(); - - int xyz = 0; - while (xyz < length) + // Get span values from the owners. + Span prefix = this.prefixOwner.GetSpan(); + Span suffix = this.suffixOwner.GetSpan(); + Span pixelStack = this.pixelStackOwner.GetSpan(); + Span buffer = this.bufferOwner.GetSpan(); + + // Cache frequently accessed instance fields into locals. + // This helps avoid repeated field loads inside the tight loop. + BufferedReadStream stream = this.stream; + int top = this.top; + int bits = this.bits; + int codeSize = this.codeSize; + int codeMask = this.codeMask; + int minCodeSize = this.minCodeSize; + int availableCode = this.availableCode; + int oldCode = this.oldCode; + int first = this.first; + int data = this.data; + int count = this.count; + int bufferIndex = this.bufferIndex; + int code = this.code; + int clearCode = this.clearCode; + int endCode = this.endCode; + + int i = 0; + while (i < length) { - if (this.top == 0) + if (top == 0) { - if (this.bits < this.codeSize) + if (bits < codeSize) { // Load bytes until there are enough bits for a code. - if (this.count == 0) + if (count == 0) { // Read a new data block. - this.count = this.ReadBlock(buffer); - if (this.count == 0) + count = ReadBlock(stream, buffer); + if (count == 0) { break; } - this.bufferIndex = 0; + bufferIndex = 0; } - this.data += buffer[this.bufferIndex] << this.bits; - - this.bits += 8; - this.bufferIndex++; - this.count--; + data += buffer[bufferIndex] << bits; + bits += 8; + bufferIndex++; + count--; continue; } // Get the next code - this.code = this.data & this.codeMask; - this.data >>= this.codeSize; - this.bits -= this.codeSize; + code = data & codeMask; + data >>= codeSize; + bits -= codeSize; // Interpret the code - if (this.code > this.availableCode || this.code == this.endCode) + if (code > availableCode || code == endCode) { break; } - if (this.code == this.clearCode) + if (code == clearCode) { // Reset the decoder - this.codeSize = this.minCodeSize + 1; - this.codeMask = (1 << this.codeSize) - 1; - this.availableCode = this.clearCode + 2; - this.oldCode = NullCode; + codeSize = minCodeSize + 1; + codeMask = (1 << codeSize) - 1; + availableCode = clearCode + 2; + oldCode = NullCode; continue; } - if (this.oldCode == NullCode) + if (oldCode == NullCode) { - Unsafe.Add(ref pixelStackRef, this.top++) = Unsafe.Add(ref suffixRef, this.code); - this.oldCode = this.code; - this.first = this.code; + pixelStack[top++] = suffix[code]; + oldCode = code; + first = code; continue; } - int inCode = this.code; - if (this.code == this.availableCode) + int inCode = code; + if (code == availableCode) { - Unsafe.Add(ref pixelStackRef, this.top++) = (byte)this.first; - - this.code = this.oldCode; + pixelStack[top++] = first; + code = oldCode; } - while (this.code > this.clearCode) + while (code > clearCode && top < MaxStackSize) { - Unsafe.Add(ref pixelStackRef, this.top++) = Unsafe.Add(ref suffixRef, this.code); - this.code = Unsafe.Add(ref prefixRef, this.code); + pixelStack[top++] = suffix[code]; + code = prefix[code]; } - int suffixCode = Unsafe.Add(ref suffixRef, this.code); - this.first = suffixCode; - Unsafe.Add(ref pixelStackRef, this.top++) = suffixCode; + int suffixCode = suffix[code]; + first = suffixCode; + pixelStack[top++] = suffixCode; - // Fix for Gifs that have "deferred clear code" as per here : + // Fix for GIFs that have "deferred clear code" as per: // https://bugzilla.mozilla.org/show_bug.cgi?id=55918 - if (this.availableCode < MaxStackSize) + if (availableCode < MaxStackSize) { - Unsafe.Add(ref prefixRef, this.availableCode) = this.oldCode; - Unsafe.Add(ref suffixRef, this.availableCode) = this.first; - this.availableCode++; - if (this.availableCode == this.codeMask + 1 && this.availableCode < MaxStackSize) + prefix[availableCode] = oldCode; + suffix[availableCode] = first; + availableCode++; + if (availableCode == codeMask + 1 && availableCode < MaxStackSize) { - this.codeSize++; - this.codeMask = (1 << this.codeSize) - 1; + codeSize++; + codeMask = (1 << codeSize) - 1; } } - this.oldCode = inCode; + oldCode = inCode; } // Pop a pixel off the pixel stack. - this.top--; + top--; - // Clear missing pixels - xyz++; + // Skip missing pixels. + i++; } + + // Write back the local values to the instance fields. + this.top = top; + this.bits = bits; + this.codeSize = codeSize; + this.codeMask = codeMask; + this.availableCode = availableCode; + this.oldCode = oldCode; + this.first = first; + this.data = data; + this.count = count; + this.bufferIndex = bufferIndex; + this.code = code; } /// /// Reads the next data block from the stream. A data block begins with a byte, /// which defines the size of the block, followed by the block itself. /// + /// The stream to read from. /// The buffer to store the block in. /// /// The . /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - private int ReadBlock(Span buffer) + private static int ReadBlock(BufferedReadStream stream, Span buffer) { - int bufferSize = this.stream.ReadByte(); + int bufferSize = stream.ReadByte(); if (bufferSize < 1) { return 0; } - int count = this.stream.Read(buffer, 0, bufferSize); + int count = stream.Read(buffer, 0, bufferSize); return count != bufferSize ? 0 : bufferSize; } @@ -378,10 +440,10 @@ private int ReadBlock(Span buffer) /// public void Dispose() { - this.prefix.Dispose(); - this.suffix.Dispose(); - this.pixelStack.Dispose(); - this.scratchBuffer.Dispose(); + this.prefixOwner.Dispose(); + this.suffixOwner.Dispose(); + this.pixelStackOwner.Dispose(); + this.bufferOwner.Dispose(); } } } diff --git a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs index 75a9800844..acc4c201b7 100644 --- a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs @@ -4,7 +4,7 @@ using System; using System.IO; using Microsoft.DotNet.RemoteExecutor; - +using Microsoft.DotNet.XUnitExtensions; using SixLabors.ImageSharp.Formats.Bmp; using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.Metadata; diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index 9ecf8ad502..4e42722d2c 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -317,5 +317,17 @@ public void IssueTooLargeLzwBits(TestImageProvider provider) image.DebugSaveMultiFrame(provider); image.CompareToReferenceOutputMultiFrame(provider, ImageComparer.Exact); } + + // https://github.com/SixLabors/ImageSharp/issues/2859 + [Theory] + [WithFile(TestImages.Gif.Issues.Issue2859_A, PixelTypes.Rgba32)] + [WithFile(TestImages.Gif.Issues.Issue2859_B, PixelTypes.Rgba32)] + public void Issue2859_LZWPixelStackOverflow(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(); + image.DebugSaveMultiFrame(provider); + image.CompareToReferenceOutputMultiFrame(provider, ImageComparer.Exact); + } } } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 1ba9ef2ef3..bb646800d0 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -464,7 +464,9 @@ public static class Issues public const string Issue1962NoColorTable = "Gif/issues/issue1962_tiniest_gif_1st.gif"; public const string Issue2012EmptyXmp = "Gif/issues/issue2012_Stronghold-Crusader-Extreme-Cover.gif"; public const string Issue2012BadMinCode = "Gif/issues/issue2012_drona1.gif"; - public const string Issue2758 = "Gif/issues/issue_2758.gif"; + public const string Issue2758 = "Gif/issues/issue_2758.gif"; + public const string Issue2859_A = "Gif/issues/issue_2859_A.gif"; + public const string Issue2859_B = "Gif/issues/issue_2859_B.gif"; } public static readonly string[] All = { Rings, Giphy, Cheers, Trans, Kumin, Leo, Ratio4x1, Ratio1x4 }; diff --git a/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2859_LZWPixelStackOverflow_Rgba32_issue_2859_A.gif/00.png b/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2859_LZWPixelStackOverflow_Rgba32_issue_2859_A.gif/00.png new file mode 100644 index 0000000000..d8c8df1263 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2859_LZWPixelStackOverflow_Rgba32_issue_2859_A.gif/00.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:daa78347749c6ff49891e2e379a373599cd35c98b453af9bf8eac52f615f935c +size 12237 diff --git a/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2859_LZWPixelStackOverflow_Rgba32_issue_2859_B.gif/00.png b/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2859_LZWPixelStackOverflow_Rgba32_issue_2859_B.gif/00.png new file mode 100644 index 0000000000..36c3683187 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2859_LZWPixelStackOverflow_Rgba32_issue_2859_B.gif/00.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:731299281f942f277ce6803e0adda3b5dd0395eb79cae26cabc9d56905fae0fd +size 1833 diff --git a/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2859_LZWPixelStackOverflow_Rgba32_issue_2859_B.gif/01.png b/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2859_LZWPixelStackOverflow_Rgba32_issue_2859_B.gif/01.png new file mode 100644 index 0000000000..c03e5817f0 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2859_LZWPixelStackOverflow_Rgba32_issue_2859_B.gif/01.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:50ccac7739142578d99a76b6d39ba377099d4a7ac30cbb0a5aee44ef1e7c9c8c +size 1271 diff --git a/tests/Images/Input/Gif/issues/issue_2859_A.gif b/tests/Images/Input/Gif/issues/issue_2859_A.gif new file mode 100644 index 0000000000..f19a047525 --- /dev/null +++ b/tests/Images/Input/Gif/issues/issue_2859_A.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:50a1a4afc62a3a36ff83596f1eb55d91cdd184c64e0d1339bbea17205c23eee1 +size 3406142 diff --git a/tests/Images/Input/Gif/issues/issue_2859_B.gif b/tests/Images/Input/Gif/issues/issue_2859_B.gif new file mode 100644 index 0000000000..109b0f8797 --- /dev/null +++ b/tests/Images/Input/Gif/issues/issue_2859_B.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:db9b2992be772a4f0ac495e994a17c7c50fb6de9794cfb9afc4a3dc26ffdfae0 +size 4543