From 72f91b8507f20b6d6f58435bbb965c91e3e81014 Mon Sep 17 00:00:00 2001
From: David Fowler <davidfowl@gmail.com>
Date: Fri, 12 Jul 2019 09:46:35 -0700
Subject: [PATCH] Correctly check for limits in the split buffer case (#12117)

- We were looking all total consumed bytes instead of bytes consumed since the last time we parsed a form value. This resulted in thinking that the key or value was too long if it couldn't be parsed after having parsed a bunch of data in the same read.
- Added tests
---
 src/Http/WebUtilities/src/FormPipeReader.cs   |  6 +-
 .../WebUtilities/test/FormPipeReaderTests.cs  | 71 ++++++++++++++++---
 2 files changed, 66 insertions(+), 11 deletions(-)

diff --git a/src/Http/WebUtilities/src/FormPipeReader.cs b/src/Http/WebUtilities/src/FormPipeReader.cs
index c8d8497807..70e85ccfcf 100644
--- a/src/Http/WebUtilities/src/FormPipeReader.cs
+++ b/src/Http/WebUtilities/src/FormPipeReader.cs
@@ -218,6 +218,7 @@ namespace Microsoft.AspNetCore.WebUtilities
         {
             var sequenceReader = new SequenceReader<byte>(buffer);
             var consumed = sequenceReader.Position;
+            var consumedBytes = default(long);
             var equalsDelimiter = GetEqualsForEncoding();
             var andDelimiter = GetAndForEncoding();
 
@@ -227,7 +228,7 @@ namespace Microsoft.AspNetCore.WebUtilities
                 if (!sequenceReader.TryReadTo(out ReadOnlySequence<byte> key, equalsDelimiter, advancePastDelimiter: false) ||
                     !sequenceReader.IsNext(equalsDelimiter, true))
                 {
-                    if (sequenceReader.Consumed > KeyLengthLimit)
+                    if ((sequenceReader.Consumed - consumedBytes) > KeyLengthLimit)
                     {
                         ThrowKeyTooLargeException();
                     }
@@ -245,7 +246,7 @@ namespace Microsoft.AspNetCore.WebUtilities
                 {
                     if (!isFinalBlock)
                     {
-                        if (sequenceReader.Consumed - key.Length > ValueLengthLimit)
+                        if ((sequenceReader.Consumed - consumedBytes - key.Length) > ValueLengthLimit)
                         {
                             ThrowValueTooLargeException();
                         }
@@ -268,6 +269,7 @@ namespace Microsoft.AspNetCore.WebUtilities
 
                 AppendAndVerify(ref accumulator, decodedKey, decodedValue);
 
+                consumedBytes = sequenceReader.Consumed;
                 consumed = sequenceReader.Position;
             }
 
diff --git a/src/Http/WebUtilities/test/FormPipeReaderTests.cs b/src/Http/WebUtilities/test/FormPipeReaderTests.cs
index 23c3be73fd..192911562f 100644
--- a/src/Http/WebUtilities/test/FormPipeReaderTests.cs
+++ b/src/Http/WebUtilities/test/FormPipeReaderTests.cs
@@ -1,6 +1,7 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System;
 using System.Buffers;
 using System.Collections.Generic;
 using System.IO;
@@ -152,15 +153,6 @@ namespace Microsoft.AspNetCore.WebUtilities
             Assert.Equal(expectedValue, form[key]);
         }
 
-        public static TheoryData<Encoding> Encodings =>
-                 new TheoryData<Encoding>
-                 {
-                     { Encoding.UTF8 },
-                     { Encoding.UTF32 },
-                     { Encoding.ASCII },
-                     { Encoding.Unicode }
-                 };
-
         [Theory]
         [MemberData(nameof(Encodings))]
         public void TryParseFormValues_SingleSegmentWorks(Encoding encoding)
@@ -386,6 +378,67 @@ namespace Microsoft.AspNetCore.WebUtilities
             }
         }
 
+        [Theory]
+        [MemberData(nameof(IncompleteFormKeys))]
+        public void ParseFormWithIncompleteKeyWhenIsFinalBlockSucceeds(ReadOnlySequence<byte> readOnlySequence)
+        {
+            KeyValueAccumulator accumulator = default;
+
+            var formReader = new FormPipeReader(null)
+            {
+                KeyLengthLimit = 3
+            };
+
+            formReader.ParseFormValues(ref readOnlySequence, ref accumulator, isFinalBlock: false);
+
+            IDictionary<string, StringValues> values = accumulator.GetResults();
+            Assert.Single(values);
+            Assert.Contains("fo", values);
+            Assert.Equal("bar", values["fo"]);
+        }
+
+        [Theory]
+        [MemberData(nameof(IncompleteFormValues))]
+        public void ParseFormWithIncompleteValueWhenIsFinalBlockSucceeds(ReadOnlySequence<byte> readOnlySequence)
+        {
+            KeyValueAccumulator accumulator = default;
+
+            var formReader = new FormPipeReader(null)
+            {
+                ValueLengthLimit = 3
+            };
+
+            formReader.ParseFormValues(ref readOnlySequence, ref accumulator, isFinalBlock: false);
+
+            IDictionary<string, StringValues> values = accumulator.GetResults();
+            Assert.Single(values);
+            Assert.Contains("fo", values);
+            Assert.Equal("bar", values["fo"]);
+        }
+
+        public static TheoryData<ReadOnlySequence<byte>> IncompleteFormKeys =>
+            new TheoryData<ReadOnlySequence<byte>>
+            {
+                { ReadOnlySequenceFactory.CreateSegments(Encoding.UTF8.GetBytes("fo=bar&b"), Encoding.UTF8.GetBytes("a"))  },
+                { new ReadOnlySequence<byte>(Encoding.UTF8.GetBytes("fo=bar&ba")) }
+            };
+
+        public static TheoryData<ReadOnlySequence<byte>> IncompleteFormValues =>
+            new TheoryData<ReadOnlySequence<byte>>
+            {
+                { ReadOnlySequenceFactory.CreateSegments(Encoding.UTF8.GetBytes("fo=bar&b"), Encoding.UTF8.GetBytes("="))  },
+                { new ReadOnlySequence<byte>(Encoding.UTF8.GetBytes("fo=bar&b=")) }
+            };
+
+        public static TheoryData<Encoding> Encodings =>
+                 new TheoryData<Encoding>
+                 {
+                     { Encoding.UTF8 },
+                     { Encoding.UTF32 },
+                     { Encoding.ASCII },
+                     { Encoding.Unicode }
+                 };
+
         internal virtual Task<Dictionary<string, StringValues>> ReadFormAsync(FormPipeReader reader)
         {
             return reader.ReadFormAsync();