From: Forest <forest@innersloth.com>
Date: Wed, 14 Oct 2020 05:42:18 +0000 (-0700)
Subject: Cleanup old, unused doc project and fix #7
X-Git-Tag: 1.0.0~26
X-Git-Url: https://git.deb.at/?a=commitdiff_plain;h=233a20748744426b8f53f90f30642a47ea37473a;p=rhonda%2Fimpostor.hazel.git

Cleanup old, unused doc project and fix #7
---

diff --git a/Hazel.UnitTests/MessageReaderTests.cs b/Hazel.UnitTests/MessageReaderTests.cs
index 04a727d..ad713ba 100644
--- a/Hazel.UnitTests/MessageReaderTests.cs
+++ b/Hazel.UnitTests/MessageReaderTests.cs
@@ -144,7 +144,6 @@ namespace Hazel.UnitTests
             msg.Write("HO");
             msg.EndMessage();
             msg.StartMessage(2);
-            msg.Write("NO");
             msg.EndMessage();
             msg.EndMessage();
 
@@ -160,9 +159,8 @@ namespace Hazel.UnitTests
             Assert.AreEqual("HO", sub.ReadString());
 
             sub = reader.ReadMessage();
-            Assert.AreEqual(3, sub.Length);
+            Assert.AreEqual(0, sub.Length);
             Assert.AreEqual(2, sub.Tag);
-            Assert.AreEqual("NO", sub.ReadString());
         }
 
         [TestMethod]
@@ -207,6 +205,47 @@ namespace Hazel.UnitTests
             catch (InvalidDataException) { }
         }
 
+        [TestMethod]
+        public void ReadMessageProtectsAgainstOverrun()
+        {
+            const string TestDataFromAPreviousPacket = "You shouldn't be able to see this data";
+            
+            // An extra byte from the length of TestData when written via MessageWriter
+            // Extra 3 bytes for the length + tag header for ReadMessage.
+            int DataLength = TestDataFromAPreviousPacket.Length + 1 + 3;
+
+            // THE BUG
+            //
+            // No bound checks. When the server wants to read a message, it
+            // reads the uint16 at that offset, treats it as a length without any bound checks.
+            // This can be allow a later ReadString or ReadBytes to create an infoleak.
+
+            MessageWriter writer = MessageWriter.Get(SendOption.None);
+
+            // This is the malicious length. No data in this message, so it should be zero.
+            writer.Write((ushort)1); 
+            writer.Write((byte)0); // Tag
+
+            // This is data from a "previous packet"
+            writer.Write(TestDataFromAPreviousPacket);
+
+            byte[] testData = writer.ToByteArray(includeHeader: false);
+
+            Assert.AreEqual(DataLength, testData.Length);
+
+            var outer = MessageReader.Get(testData);
+
+            // Length is just the malicious message header.
+            outer.Length = 3;
+
+            try
+            {
+                outer.ReadMessage();
+                Assert.Fail("ReadMessage is expected to throw");
+            }
+            catch (InvalidDataException) { }
+        }
+
         [TestMethod]
         public void GetLittleEndian()
         {
diff --git a/Hazel.sln b/Hazel.sln
index 51a6565..bc27b0a 100644
--- a/Hazel.sln
+++ b/Hazel.sln
@@ -1,7 +1,7 @@
 
 Microsoft Visual Studio Solution File, Format Version 12.00
 # Visual Studio Version 16
-VisualStudioVersion = 16.0.30621.155
+VisualStudioVersion = 16.0.30523.141
 MinimumVisualStudioVersion = 10.0.40219.1
 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Hazel", "Hazel\Hazel.csproj", "{02CFBD30-D77D-400F-94B2-700F60EFDD7F}"
 EndProject
diff --git a/Hazel/MessageReader.cs b/Hazel/MessageReader.cs
index e19d90a..cc3ef2a 100644
--- a/Hazel/MessageReader.cs
+++ b/Hazel/MessageReader.cs
@@ -108,7 +108,7 @@ namespace Hazel
         public MessageReader ReadMessage()
         {
             // Ensure there is at least a header
-            if (this.readHead + 3 > this.Buffer.Length) return null;
+            if (this.BytesRemaining < 3) throw new InvalidDataException($"ReadMessage header is longer than message length: 3 of {this.BytesRemaining}");
 
             var output = new MessageReader();
 
@@ -122,6 +122,8 @@ namespace Hazel
             output.Offset += 3;
             output.Position = 0;
 
+            if (this.BytesRemaining < output.Length + 3) throw new InvalidDataException($"Message length is longer than message length: {output.Length + 3} of {this.BytesRemaining}");
+
             this.Position += output.Length + 3;
             return output;
         }
diff --git a/Hazel/Properties/AssemblyInfo.cs b/Hazel/Properties/AssemblyInfo.cs
index 0e00915..3d0a292 100644
--- a/Hazel/Properties/AssemblyInfo.cs
+++ b/Hazel/Properties/AssemblyInfo.cs
@@ -10,7 +10,7 @@ using System.Runtime.InteropServices;
 [assembly: AssemblyConfiguration("")]
 [assembly: AssemblyCompany("")]
 [assembly: AssemblyProduct("Hazel")]
-[assembly: AssemblyCopyright("Copyright ©  2019")]
+[assembly: AssemblyCopyright("Copyright © 2020")]
 [assembly: AssemblyTrademark("")]
 [assembly: AssemblyCulture("")]
 
@@ -29,14 +29,8 @@ using System.Runtime.InteropServices;
 //      Build Number
 //      Revision
 //
-// You can specify all the values or you can default the Build and Revision Numbers 
-// by using the '*' as shown below:
-// [assembly: AssemblyVersion("1.0.*")]
-[assembly: AssemblyVersion("1.0.0.0")]
+[assembly: AssemblyVersion("1.0.1.0")]
 [assembly: AssemblyFileVersion("1.0.0.0")]
 
-// NuGet version information
-[assembly: AssemblyInformationalVersion("0.1.2-beta")]
-
 // Show internals to unit testing assembly so it can test
 [assembly:InternalsVisibleTo("Hazel.UnitTests")]
\ No newline at end of file