Skip to content

Commit

Permalink
Don't read past end of scratch buffer (#26)
Browse files Browse the repository at this point in the history
When the incoming input buffer is short, we must make sure that we
properly set inputLimitMinMaxTagLength to prevent buffer overruns.
Otherwise we'll read past the end of the buffer.

Close #25
  • Loading branch information
brantburnett authored Nov 3, 2020
1 parent c9ddcf0 commit 6876d6e
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 2 deletions.
34 changes: 34 additions & 0 deletions Snappier.Tests/Internal/SnappyDecompressorTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using Snappier.Internal;
using Xunit;

namespace Snappier.Tests.Internal
{
public class SnappyDecompressorTests
{
#region DecompressAllTags

[Fact]
public void DecompressAllTags_ShortInputBufferWhichCopiesToScratch_DoesNotReadPastEndOfScratch()
{
// Arrange

var decompressor = new SnappyDecompressor();
decompressor.SetExpectedLengthForTest(1024);

decompressor.WriteToBufferForTest(Enumerable.Range(0, 255).Select(p => (byte) p).ToArray());

// if in error, decompressor will read the 222, 0, 0 as the next tag and throw a copy offset exception
decompressor.LoadScratchForTest(new byte[] { 222, 222, 222, 222, 0, 0 }, 0);

// Act

decompressor.DecompressAllTags(new byte[] { 150, 255, 0 });
}

#endregion
}
}
37 changes: 35 additions & 2 deletions Snappier/Internal/SnappyDecompressor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace Snappier.Internal
{
internal sealed class SnappyDecompressor : IDisposable
{
private readonly byte[] _scratch = new byte[Constants.MaximumTagLength];
private byte[] _scratch = new byte[Constants.MaximumTagLength];
private int _scratchLength = 0;

private int _remainingLiteral;
Expand Down Expand Up @@ -176,7 +176,7 @@ public static int ReadUncompressedLength(ReadOnlySpan<byte> input)
return result;
}

private unsafe void DecompressAllTags(ReadOnlySpan<byte> inputSpan)
internal unsafe void DecompressAllTags(ReadOnlySpan<byte> inputSpan)
{
// Put Constants.CharTable on the stack to simplify lookups within the loops below
ReadOnlySpan<ushort> charTable = Constants.CharTable.AsSpan();
Expand Down Expand Up @@ -278,6 +278,9 @@ private unsafe void DecompressAllTags(ReadOnlySpan<byte> inputSpan)
{
goto exit;
}

inputLimitMinMaxTagLength = inputEnd - Math.Min(inputEnd - input,
Constants.MaximumTagLength - 1);
}

uint preload = Helpers.UnsafeReadUInt32(input);
Expand Down Expand Up @@ -604,6 +607,36 @@ public int Read(Span<byte> destination)

#endregion

#region Test Helpers

/// <summary>
/// Load some data into the output buffer, only used for testing.
/// </summary>
/// <param name="toWrite"></param>
internal void WriteToBufferForTest(ReadOnlySpan<byte> toWrite)
{
Append(toWrite);
}

/// <summary>
/// Load a byte array into _scratch, only used for testing.
/// </summary>
internal void LoadScratchForTest(byte[] newScratch, int newScratchLength)
{
_scratch = newScratch ?? throw new ArgumentNullException(nameof(newScratch));
_scratchLength = newScratchLength;
}

/// <summary>
/// Only used for testing.
/// </summary>
internal void SetExpectedLengthForTest(int expectedLength)
{
ExpectedLength = expectedLength;
}

#endregion

public void Dispose()
{
_lookbackBufferOwner?.Dispose();
Expand Down

0 comments on commit 6876d6e

Please sign in to comment.