From 6876d6eb5879d21dd594566fd60772f4e3402804 Mon Sep 17 00:00:00 2001
From: Brant Burnett <bburnett@centeredgesoftware.com>
Date: Mon, 2 Nov 2020 20:26:08 -0500
Subject: [PATCH] Don't read past end of scratch buffer (#26)

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
---
 .../Internal/SnappyDecompressorTests.cs       | 34 +++++++++++++++++
 Snappier/Internal/SnappyDecompressor.cs       | 37 ++++++++++++++++++-
 2 files changed, 69 insertions(+), 2 deletions(-)
 create mode 100644 Snappier.Tests/Internal/SnappyDecompressorTests.cs

diff --git a/Snappier.Tests/Internal/SnappyDecompressorTests.cs b/Snappier.Tests/Internal/SnappyDecompressorTests.cs
new file mode 100644
index 0000000..78b566e
--- /dev/null
+++ b/Snappier.Tests/Internal/SnappyDecompressorTests.cs
@@ -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
+    }
+}
diff --git a/Snappier/Internal/SnappyDecompressor.cs b/Snappier/Internal/SnappyDecompressor.cs
index 6b74845..bb345bb 100644
--- a/Snappier/Internal/SnappyDecompressor.cs
+++ b/Snappier/Internal/SnappyDecompressor.cs
@@ -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;
@@ -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();
@@ -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);
@@ -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();