Add some exceptions for MessageParser.ParseFrom (#5588)

* Fix #5513

* Added tests for invalid lengths when reading strings and bytes.
Added test for reading tags with invalid wire types in unknown field set.
Changed invalid length check in ReadString to match the one in ReadBytes

csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs

@@ -373,6 +373,42 @@ namespace Google.Protobuf
             Assert.AreEqual('\ufffd', text[0]);
         }
 
+        [Test]
+        public void ReadNegativeSizedStringThrowsInvalidProtocolBufferException()
+        {
+            MemoryStream ms = new MemoryStream();
+            CodedOutputStream output = new CodedOutputStream(ms);
+
+            uint tag = WireFormat.MakeTag(1, WireFormat.WireType.LengthDelimited);
+            output.WriteRawVarint32(tag);
+            output.WriteLength(-1);
+            output.Flush();
+            ms.Position = 0;
+
+            CodedInputStream input = new CodedInputStream(ms);
+
+            Assert.AreEqual(tag, input.ReadTag());
+            Assert.Throws<InvalidProtocolBufferException>(() => input.ReadString());
+        }
+
+        [Test]
+        public void ReadNegativeSizedBytesThrowsInvalidProtocolBufferException()
+        {
+            MemoryStream ms = new MemoryStream();
+            CodedOutputStream output = new CodedOutputStream(ms);
+
+            uint tag = WireFormat.MakeTag(1, WireFormat.WireType.LengthDelimited);
+            output.WriteRawVarint32(tag);
+            output.WriteLength(-1);
+            output.Flush();
+            ms.Position = 0;
+
+            CodedInputStream input = new CodedInputStream(ms);
+
+            Assert.AreEqual(tag, input.ReadTag());
+            Assert.Throws<InvalidProtocolBufferException>(() => input.ReadBytes());
+        }
+
         /// <summary>
         /// A stream which limits the number of bytes it reads at a time.
         /// We use this to make sure that CodedInputStream doesn't screw up when

csharp/src/Google.Protobuf.Test/UnknownFieldSetTest.cs

@@ -172,5 +172,23 @@ namespace Google.Protobuf
             assertEmpty(discardingParser1.ParseFrom(new MemoryStream(data)));
             assertEmpty(discardingParser2.ParseFrom(new MemoryStream(data)));
         }
+
+        [Test]
+        public void TestReadInvalidWireTypeThrowsInvalidProtocolBufferException()
+        {
+            MemoryStream ms = new MemoryStream();
+            CodedOutputStream output = new CodedOutputStream(ms);
+
+            uint tag = WireFormat.MakeTag(1, (WireFormat.WireType)6);
+            output.WriteRawVarint32(tag);
+            output.WriteLength(-1);
+            output.Flush();
+            ms.Position = 0;
+
+            CodedInputStream input = new CodedInputStream(ms);
+            Assert.AreEqual(tag, input.ReadTag());
+
+            Assert.Throws<InvalidProtocolBufferException>(() => UnknownFieldSet.MergeFieldFrom(null, input));
+        }
     }
 }

csharp/src/Google.Protobuf/CodedInputStream.cs

@@ -560,7 +560,7 @@ namespace Google.Protobuf
             {
                 return "";
             }
-            if (length <= bufferSize - bufferPos)
+            if (length <= bufferSize - bufferPos && length > 0)
             {
                 // Fast path:  We already have the bytes in a contiguous buffer, so
                 //   just copy directly from it.

csharp/src/Google.Protobuf/InvalidProtocolBufferException.cs

@@ -88,6 +88,12 @@ namespace Google.Protobuf
                 "Protocol message contained an invalid tag (zero).");
         }
 
+        internal static InvalidProtocolBufferException InvalidWireType()
+        {
+            return new InvalidProtocolBufferException(
+                "Protocol message contained a tag with an invalid wire type.");
+        }
+
         internal static InvalidProtocolBufferException InvalidBase64(Exception innerException)
         {
             return new InvalidProtocolBufferException("Invalid base64 data", innerException);

csharp/src/Google.Protobuf/UnknownFieldSet.cs

@@ -229,7 +229,7 @@ namespace Google.Protobuf
                         return false;
                     }
                 default:
-                    throw new InvalidOperationException("Wire Type is invalid.");
+                    throw InvalidProtocolBufferException.InvalidWireType();
             }
         }