From bd6faa5ca1f43f78ea708a92d3ff44608e23381b Mon Sep 17 00:00:00 2001
From: David Fowler <davidfowl@gmail.com>
Date: Fri, 8 Mar 2019 09:42:02 -0800
Subject: [PATCH] Chunked request parsing does not properly update examined
 (#8318)

- The chunked parsing logic didn't properly update the examined position when parsing the chunked prefix. This started to throw because Pipe now throws if examined is set to the position before the previous.
---
 eng/Version.Details.xml                       |  2 +-
 eng/Versions.props                            |  2 +-
 .../Http/Http1ChunkedEncodingMessageBody.cs   |  6 ++
 .../Kestrel/Core/test/MessageBodyTests.cs     | 88 +++++++++++++++++++
 4 files changed, 96 insertions(+), 2 deletions(-)

diff --git a/eng/Version.Details.xml b/eng/Version.Details.xml
index b41add8142..f9e32d91eb 100644
--- a/eng/Version.Details.xml
+++ b/eng/Version.Details.xml
@@ -305,7 +305,7 @@
       <Uri>https://github.com/dotnet/corefx</Uri>
       <Sha>00000</Sha>
     </Dependency>
-    <Dependency Name="System.IO.Pipelines" Version="4.6.0-preview4.19127.11">
+    <Dependency Name="System.IO.Pipelines" Version="4.6.0-preview4.19156.2">
       <Uri>https://github.com/dotnet/corefx</Uri>
       <Sha>00000</Sha>
     </Dependency>
diff --git a/eng/Versions.props b/eng/Versions.props
index d7bd40e97a..e7d82c184c 100644
--- a/eng/Versions.props
+++ b/eng/Versions.props
@@ -29,7 +29,7 @@
     <SystemComponentModelAnnotationsPackageVersion>4.6.0-preview4.19156.2</SystemComponentModelAnnotationsPackageVersion>
     <SystemDataSqlClientPackageVersion>4.7.0-preview4.19156.2</SystemDataSqlClientPackageVersion>
     <SystemDiagnosticsEventLogPackageVersion>4.6.0-preview4.19156.2</SystemDiagnosticsEventLogPackageVersion>
-    <SystemIOPipelinesPackageVersion>4.6.0-preview4.19127.11</SystemIOPipelinesPackageVersion>
+    <SystemIOPipelinesPackageVersion>4.6.0-preview4.19156.2</SystemIOPipelinesPackageVersion>
     <SystemNetHttpWinHttpHandlerPackageVersion>4.6.0-preview4.19156.2</SystemNetHttpWinHttpHandlerPackageVersion>
     <SystemNetWebSocketsWebSocketProtocolPackageVersion>4.6.0-preview4.19156.2</SystemNetWebSocketsWebSocketProtocolPackageVersion>
     <SystemReflectionMetadataPackageVersion>1.7.0-preview4.19156.2</SystemReflectionMetadataPackageVersion>
diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs
index 963fd5d85c..4f7a15826d 100644
--- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs
+++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs
@@ -323,6 +323,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                 return;
             }
 
+            // Assigned this before calculating the chunk size since that can throw
+            examined = reader.Position;
+
             var chunkSize = CalculateChunkSize(ch1, 0);
             ch1 = ch2;
 
@@ -360,6 +363,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                 ch1 = ch2;
             }
 
+            // Set examined so that we capture the progress that way made
+            examined = reader.Position;
+
             // At this point, 10 bytes have been consumed which is enough to parse the max value "7FFFFFFF\r\n".
             BadHttpRequestException.Throw(RequestRejectionReason.BadChunkSizeData);
         }
diff --git a/src/Servers/Kestrel/Core/test/MessageBodyTests.cs b/src/Servers/Kestrel/Core/test/MessageBodyTests.cs
index 785097ab17..bd11ad9d81 100644
--- a/src/Servers/Kestrel/Core/test/MessageBodyTests.cs
+++ b/src/Servers/Kestrel/Core/test/MessageBodyTests.cs
@@ -198,6 +198,94 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             }
         }
 
+        [Fact]
+        public async Task BadChunkPrefixThrowsBadRequestException()
+        {
+            using (var input = new TestInput())
+            {
+                var body = Http1MessageBody.For(HttpVersion.Http11, new HttpRequestHeaders { HeaderTransferEncoding = "chunked" }, input.Http1Connection);
+                var mockBodyControl = new Mock<IHttpBodyControlFeature>();
+                mockBodyControl.Setup(m => m.AllowSynchronousIO).Returns(true);
+                var reader = new HttpRequestPipeReader();
+                var stream = new HttpRequestStream(mockBodyControl.Object, reader);
+                reader.StartAcceptingReads(body);
+                var buffer = new byte[1024];
+                var task = stream.ReadAsync(buffer, 0, buffer.Length);
+
+                input.Add("g");
+                input.Add("g");
+
+                await Assert.ThrowsAsync<BadHttpRequestException>(() => task);
+
+                await body.StopAsync();
+            }
+        }
+
+        [Fact]
+        public async Task WritingChunkOverMaxChunkSizeThrowsBadRequest()
+        {
+            using (var input = new TestInput())
+            {
+                var body = Http1MessageBody.For(HttpVersion.Http11, new HttpRequestHeaders { HeaderTransferEncoding = "chunked" }, input.Http1Connection);
+                var mockBodyControl = new Mock<IHttpBodyControlFeature>();
+                mockBodyControl.Setup(m => m.AllowSynchronousIO).Returns(true);
+                var reader = new HttpRequestPipeReader();
+                var stream = new HttpRequestStream(mockBodyControl.Object, reader);
+                reader.StartAcceptingReads(body);
+                var buffer = new byte[1024];
+                var task = stream.ReadAsync(buffer, 0, buffer.Length);
+
+                // Max is 10 bytes
+                for (int i = 0; i < 11; i++)
+                {
+                    input.Add(i.ToString());
+                }
+
+                await Assert.ThrowsAsync<BadHttpRequestException>(() => task);
+
+                await body.StopAsync();
+            }
+        }
+
+        [Fact]
+        public async Task InvalidChunkSuffixThrowsBadRequest()
+        {
+            using (var input = new TestInput())
+            {
+                var body = Http1MessageBody.For(HttpVersion.Http11, new HttpRequestHeaders { HeaderTransferEncoding = "chunked" }, input.Http1Connection);
+                var mockBodyControl = new Mock<IHttpBodyControlFeature>();
+                mockBodyControl.Setup(m => m.AllowSynchronousIO).Returns(true);
+                var reader = new HttpRequestPipeReader();
+                var stream = new HttpRequestStream(mockBodyControl.Object, reader);
+                reader.StartAcceptingReads(body);
+                var buffer = new byte[1024];
+
+                async Task ReadAsync()
+                {
+                    while (true)
+                    {
+                        await stream.ReadAsync(buffer, 0, buffer.Length);
+                    }
+                }
+
+                var task = ReadAsync();
+
+                input.Add("1");
+                input.Add("\r");
+                input.Add("\n");
+                input.Add("h");
+                input.Add("0");
+                input.Add("\r");
+                input.Add("\n");
+                input.Add("\r");
+                input.Add("n");
+
+                await Assert.ThrowsAsync<BadHttpRequestException>(() => task);
+
+                await body.StopAsync();
+            }
+        }
+
         [Fact]
         public async Task CanReadAsyncFromChunkedEncoding()
         {