Skip to content

Commit

Permalink
Merge pull request #804 from SixLabors/js/fix798
Browse files Browse the repository at this point in the history
Use bounds checks in Huffman ctr. Fix #798
  • Loading branch information
JimBobSquarePants authored Jan 9, 2019
2 parents 441942b + 93bb373 commit 5755779
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 14 deletions.
23 changes: 13 additions & 10 deletions src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,10 @@ internal unsafe struct HuffmanTable
/// <param name="values">The huffman values</param>
public HuffmanTable(MemoryAllocator memoryAllocator, ReadOnlySpan<byte> codeLengths, ReadOnlySpan<byte> values)
{
const int Length = 257;
using (IMemoryOwner<short> huffcode = memoryAllocator.Allocate<short>(Length))
// We do some bounds checks in the code here to protect against AccessViolationExceptions
const int HuffCodeLength = 257;
const int MaxSizeLength = HuffCodeLength - 1;
using (IMemoryOwner<short> huffcode = memoryAllocator.Allocate<short>(HuffCodeLength))
{
ref short huffcodeRef = ref MemoryMarshal.GetReference(huffcode.GetSpan());
ref byte codeLengthsRef = ref MemoryMarshal.GetReference(codeLengths);
Expand All @@ -63,7 +65,7 @@ public HuffmanTable(MemoryAllocator memoryAllocator, ReadOnlySpan<byte> codeLeng
for (short i = 1; i < 17; i++)
{
byte length = Unsafe.Add(ref codeLengthsRef, i);
for (short j = 0; j < length; j++)
for (short j = 0; j < length && x < MaxSizeLength; j++)
{
Unsafe.Add(ref sizesRef, x++) = i;
}
Expand All @@ -84,7 +86,7 @@ public HuffmanTable(MemoryAllocator memoryAllocator, ReadOnlySpan<byte> codeLeng
Unsafe.Add(ref valOffsetRef, k) = (int)(si - code);
if (Unsafe.Add(ref sizesRef, si) == k)
{
while (Unsafe.Add(ref sizesRef, si) == k)
while (Unsafe.Add(ref sizesRef, si) == k && si < HuffCodeLength)
{
Unsafe.Add(ref huffcodeRef, si++) = (short)code++;
}
Expand All @@ -100,19 +102,20 @@ public HuffmanTable(MemoryAllocator memoryAllocator, ReadOnlySpan<byte> codeLeng

// Generate non-spec lookup tables to speed up decoding.
const int FastBits = ScanDecoder.FastBits;
ref byte fastRef = ref this.Lookahead[0];
Unsafe.InitBlockUnaligned(ref fastRef, 0xFF, 1 << FastBits); // Flag for non-accelerated
ref byte lookaheadRef = ref this.Lookahead[0];
const uint MaxFastLength = 1 << FastBits;
Unsafe.InitBlockUnaligned(ref lookaheadRef, 0xFF, MaxFastLength); // Flag for non-accelerated

for (int i = 0; i < si; i++)
{
int size = Unsafe.Add(ref sizesRef, i);
if (size <= FastBits)
{
int c = Unsafe.Add(ref huffcodeRef, i) << (FastBits - size);
int m = 1 << (FastBits - size);
for (int l = 0; l < m; l++)
int huffCode = Unsafe.Add(ref huffcodeRef, i) << (FastBits - size);
int max = 1 << (FastBits - size);
for (int left = 0; left < max; left++)
{
Unsafe.Add(ref fastRef, c + l) = (byte)i;
Unsafe.Add(ref lookaheadRef, huffCode + left) = (byte)i;
}
}
}
Expand Down
10 changes: 6 additions & 4 deletions tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Baseline.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,11 @@ public void DecodeBaselineJpeg_CriticalEOF_ShouldThrow<TPixel>(TestImageProvider
[Theory]
[WithFile(TestImages.Jpeg.Issues.InvalidJpegThrowsWrongException797, PixelTypes.Rgba32)]
public void LoadingImage_InvalidTagLength_ShouldThrow<TPixel>(TestImageProvider<TPixel> provider)
where TPixel : struct, IPixel<TPixel>
{
Assert.Throws<ImageFormatException>(() => provider.GetImage());
}
where TPixel : struct, IPixel<TPixel> => Assert.Throws<ImageFormatException>(() => provider.GetImage());

[Theory]
[WithFile(TestImages.Jpeg.Issues.AccessViolationException798, PixelTypes.Rgba32)]
public void LoadingImage_BadHuffman_ShouldNotThrow<TPixel>(TestImageProvider<TPixel> provider)
where TPixel : struct, IPixel<TPixel> => Assert.NotNull(provider.GetImage());
}
}
1 change: 1 addition & 0 deletions tests/ImageSharp.Tests/TestImages.cs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ public static class Issues
public const string ExifGetString750Transform = "Jpg/issues/issue750-exif-tranform.jpg";
public const string ExifGetString750Load = "Jpg/issues/issue750-exif-load.jpg";
public const string InvalidJpegThrowsWrongException797 = "Jpg/issues/Issue797-InvalidImage.jpg";
public const string AccessViolationException798 = "Jpg/issues/Issue798-AccessViolationException.jpg";
}

public static readonly string[] All = Baseline.All.Concat(Progressive.All).ToArray();
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit 5755779

Please sign in to comment.