From 864df890a7639dae59a8ccad06a767adeb7210a3 Mon Sep 17 00:00:00 2001 From: John Brock Date: Wed, 24 Jan 2018 17:22:18 -0800 Subject: [PATCH] Remove 64MB memory limit when deserializing messages in C# Increased `CodedInputStream.DefaultSizeLimit` to `Int32.MaxValue` to make it consistent with the Java implementation. --- .../CodedInputStreamTest.cs | 83 +++++++++++++++++++ .../src/Google.Protobuf/CodedInputStream.cs | 6 +- 2 files changed, 86 insertions(+), 3 deletions(-) mode change 100644 => 100755 csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs diff --git a/csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs b/csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs old mode 100644 new mode 100755 index e7c6b80596..8795fa65f4 --- a/csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs +++ b/csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs @@ -615,5 +615,88 @@ namespace Google.Protobuf var stream = new CodedInputStream(new byte[10]); stream.Dispose(); } + + [Test] + public void TestParseMessagesCloseTo2G() + { + byte[] serializedMessage = GenerateBigSerializedMessage(); + // How many of these big messages do we need to take us near our 2GB limit? + int count = Int32.MaxValue / serializedMessage.Length; + // Now make a MemoryStream that will fake a near-2GB stream of messages by returning + // our big serialized message 'count' times. + using (RepeatingMemoryStream stream = new RepeatingMemoryStream(serializedMessage, count)) + { + Assert.DoesNotThrow(()=>TestAllTypes.Parser.ParseFrom(stream)); + } + } + + [Test] + public void TestParseMessagesOver2G() + { + byte[] serializedMessage = GenerateBigSerializedMessage(); + // How many of these big messages do we need to take us near our 2GB limit? + int count = Int32.MaxValue / serializedMessage.Length; + // Now add one to take us over the 2GB limit + count++; + // Now make a MemoryStream that will fake a near-2GB stream of messages by returning + // our big serialized message 'count' times. + using (RepeatingMemoryStream stream = new RepeatingMemoryStream(serializedMessage, count)) + { + Assert.Throws(() => TestAllTypes.Parser.ParseFrom(stream), + "Protocol message was too large. May be malicious. " + + "Use CodedInputStream.SetSizeLimit() to increase the size limit."); + } + } + + /// A serialized big message + private static byte[] GenerateBigSerializedMessage() + { + byte[] value = new byte[16 * 1024 * 1024]; + TestAllTypes message = SampleMessages.CreateFullTestAllTypes(); + message.SingleBytes = ByteString.CopyFrom(value); + return message.ToByteArray(); + } + + /// + /// A MemoryStream that repeats a byte arrays' content a number of times. + /// Simulates really large input without consuming loads of memory. Used above + /// to test the parsing behavior when the input size exceeds 2GB or close to it. + /// + private class RepeatingMemoryStream: MemoryStream + { + private readonly byte[] bytes; + private readonly int maxIterations; + private int index = 0; + + public RepeatingMemoryStream(byte[] bytes, int maxIterations) + { + this.bytes = bytes; + this.maxIterations = maxIterations; + } + + public override int Read(byte[] buffer, int offset, int count) + { + if (bytes.Length == 0) + { + return 0; + } + int numBytesCopiedTotal = 0; + while (numBytesCopiedTotal < count && index < maxIterations) + { + int numBytesToCopy = Math.Min(bytes.Length - (int)Position, count); + Array.Copy(bytes, (int)Position, buffer, offset, numBytesToCopy); + numBytesCopiedTotal += numBytesToCopy; + offset += numBytesToCopy; + count -= numBytesCopiedTotal; + Position += numBytesToCopy; + if (Position >= bytes.Length) + { + Position = 0; + index++; + } + } + return numBytesCopiedTotal; + } + } } } \ No newline at end of file diff --git a/csharp/src/Google.Protobuf/CodedInputStream.cs b/csharp/src/Google.Protobuf/CodedInputStream.cs index 6bee238f7e..0a829545e2 100644 --- a/csharp/src/Google.Protobuf/CodedInputStream.cs +++ b/csharp/src/Google.Protobuf/CodedInputStream.cs @@ -94,7 +94,7 @@ namespace Google.Protobuf private bool hasNextTag = false; internal const int DefaultRecursionLimit = 64; - internal const int DefaultSizeLimit = 64 << 20; // 64MB + internal const int DefaultSizeLimit = Int32.MaxValue; internal const int BufferSize = 4096; /// @@ -248,7 +248,7 @@ namespace Google.Protobuf /// /// This limit is applied when reading from the underlying stream, as a sanity check. It is /// not applied when reading from a byte array data source without an underlying stream. - /// The default value is 64MB. + /// The default value is Int32.MaxValue. /// /// /// The size limit. @@ -1058,7 +1058,7 @@ namespace Google.Protobuf RecomputeBufferSizeAfterLimit(); int totalBytesRead = totalBytesRetired + bufferSize + bufferSizeAfterLimit; - if (totalBytesRead > sizeLimit || totalBytesRead < 0) + if (totalBytesRead < 0 || totalBytesRead > sizeLimit) { throw InvalidProtocolBufferException.SizeLimitExceeded(); }