Aborting request if a string can't be converted to ASCII

src/Microsoft.AspNetCore.Server.Kestrel/Http/Frame.cs

@@ -43,7 +43,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Http
         private readonly object _onStartingSync = new Object();
         private readonly object _onCompletedSync = new Object();
 
-        private bool _requestRejected;
+        protected bool _requestRejected;
         private Streams _frameStreams;
 
         protected List<KeyValuePair<Func<object, Task>, object>> _onStarting;
@@ -1175,12 +1175,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Http
         }
 
         public void RejectRequest(string message)
+        {
+            var ex = new BadHttpRequestException(message);
+            SetBadRequestState(ex);
+            throw ex;
+        }
+
+        public void SetBadRequestState(BadHttpRequestException ex)
         {
             _requestProcessingStopping = true;
             _requestRejected = true;
-            var ex = new BadHttpRequestException(message);
             Log.ConnectionBadRequest(ConnectionId, ex);
-            throw ex;
         }
 
         protected void ReportApplicationError(Exception ex)

src/Microsoft.AspNetCore.Server.Kestrel/Http/FrameOfT.cs

@@ -5,6 +5,7 @@ using System;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Hosting.Server;
+using Microsoft.AspNetCore.Server.Kestrel.Exceptions;
 using Microsoft.Extensions.Logging;
 
 namespace Microsoft.AspNetCore.Server.Kestrel.Http
@@ -145,6 +146,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Http
                     Reset();
                 }
             }
+            catch (BadHttpRequestException ex)
+            {
+                if (!_requestRejected)
+                {
+                    SetBadRequestState(ex);
+                    Log.LogWarning(0, ex, "Connection processing ended abnormally");
+                }
+            }
             catch (Exception ex)
             {
                 Log.LogWarning(0, ex, "Connection processing ended abnormally");

src/Microsoft.AspNetCore.Server.Kestrel/Infrastructure/AsciiUtilities.cs

@@ -5,11 +5,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Infrastructure
 {
     internal class AsciiUtilities
     {
-        public static unsafe void GetAsciiString(byte* input, char* output, int count)
+        public static unsafe bool TryGetAsciiString(byte* input, char* output, int count)
         {
             var i = 0;
+            int orValue = 0;
+            bool hasZero = false;
             while (i < count - 11)
             {
+                orValue |= *input | *(input + 1) | *(input + 2) | *(input + 3) | *(input + 4) | *(input + 5) |
+                    *(input + 6) | *(input + 7) | *(input + 8) | *(input + 9) | *(input + 10) | *(input + 11);
+                hasZero = hasZero || *input == 0 || *(input + 1) == 0 || *(input + 2) == 0 || *(input + 3) == 0 ||
+                    *(input + 4) == 0 || *(input + 5) == 0 || *(input + 6) == 0 || *(input + 7) == 0 ||
+                    *(input + 8) == 0 || *(input + 9) == 0 || *(input + 10) == 0 || *(input + 11) == 0;
+
                 i += 12;
                 *(output) = (char)*(input);
                 *(output + 1) = (char)*(input + 1);
@@ -28,6 +36,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Infrastructure
             }
             if (i < count - 5)
             {
+                orValue |= *input | *(input + 1) | *(input + 2) | *(input + 3) | *(input + 4) | *(input + 5);
+                hasZero = hasZero || *input == 0 || *(input + 1) == 0 || *(input + 2) == 0 || *(input + 3) == 0 ||
+                    *(input + 4) == 0 || *(input + 5) == 0;
+
                 i += 6;
                 *(output) = (char)*(input);
                 *(output + 1) = (char)*(input + 1);
@@ -40,6 +52,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Infrastructure
             }
             if (i < count - 3)
             {
+                orValue |= *input | *(input + 1) | *(input + 2) | *(input + 3);
+                hasZero = hasZero || *input == 0 || *(input + 1) == 0 || *(input + 2) == 0 || *(input + 3) == 0;
+
                 i += 4;
                 *(output) = (char)*(input);
                 *(output + 1) = (char)*(input + 1);
@@ -48,13 +63,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Infrastructure
                 output += 4;
                 input += 4;
             }
+
             while (i < count)
             {
+                orValue |= *input;
+                hasZero = hasZero || *input == 0;
                 i++;
                 *output = (char)*input;
                 output++;
                 input++;
             }
+
+            return (orValue & 0x80) == 0 && !hasZero;
         }
     }
 }

src/Microsoft.AspNetCore.Server.Kestrel/Infrastructure/MemoryPoolIteratorExtensions.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Diagnostics;
 using System.Text;
+using Microsoft.AspNetCore.Server.Kestrel.Exceptions;
 
 namespace Microsoft.AspNetCore.Server.Kestrel.Infrastructure
 {
@@ -93,10 +94,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Infrastructure
                 return null;
             }
 
-            // Bytes out of the range of ascii are treated as "opaque data"
-            // and kept in string as a char value that casts to same input byte value
-            // https://tools.ietf.org/html/rfc7230#section-3.2.4
-
             var inputOffset = start.Index;
             var block = start.Block;
 
@@ -117,7 +114,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Infrastructure
 
                     if (following > 0)
                     {
-                        AsciiUtilities.GetAsciiString(block.DataFixedPtr + inputOffset, output + outputOffset, following);
+                        if (!AsciiUtilities.TryGetAsciiString(block.DataFixedPtr + inputOffset, output + outputOffset, following))
+                        {
+                            throw new BadHttpRequestException("The input string contains non-ASCII or null characters.");
+                        }
+
                         outputOffset += following;
                         remaining -= following;
                     }

test/Microsoft.AspNetCore.Server.KestrelTests/AsciiDecoding.cs

@@ -3,17 +3,18 @@
 
 using System;
 using System.Linq;
+using Microsoft.AspNetCore.Server.Kestrel.Exceptions;
 using Microsoft.AspNetCore.Server.Kestrel.Infrastructure;
 using Xunit;
 
 namespace Microsoft.AspNetCore.Server.KestrelTests
 {
-    public class AsciiDecoderTests
+    public class AsciiDecodingTests
     {
         [Fact]
-        private void FullByteRangeSupported()
+        private void FullAsciiRangeSupported()
         {
-            var byteRange = Enumerable.Range(0, 256).Select(x => (byte)x).ToArray();
+            var byteRange = Enumerable.Range(1, 127).Select(x => (byte)x).ToArray();
             using (var pool = new MemoryPool())
             {
                 var mem = pool.Lease();
@@ -26,7 +27,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
 
                 Assert.Equal(s.Length, byteRange.Length);
 
-                for (var i = 0; i < byteRange.Length; i++)
+                for (var i = 1; i < byteRange.Length; i++)
                 {
                     var sb = (byte)s[i];
                     var b = byteRange[i];
@@ -38,10 +39,36 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
             }
         }
 
+        [Theory]
+        [InlineData(0x00)]
+        [InlineData(0x80)]
+        private void ExceptionThrownForZeroOrNonAscii(byte b)
+        {
+            for (var length = 1; length < 16; length++)
+            {
+                for (var position = 0; position < length; position++)
+                {
+                    var byteRange = Enumerable.Range(1, length).Select(x => (byte)x).ToArray();
+                    byteRange[position] = b;
+
+                    using (var pool = new MemoryPool())
+                    {
+                        var mem = pool.Lease();
+                        mem.GetIterator().CopyFrom(byteRange);
+
+                        var begin = mem.GetIterator();
+                        var end = GetIterator(begin, byteRange.Length);
+
+                        Assert.Throws<BadHttpRequestException>(() => begin.GetAsciiString(end));
+                    }
+                }
+            }
+        }
+
         [Fact]
         private void MultiBlockProducesCorrectResults()
         {
-            var byteRange = Enumerable.Range(0, 512 + 64).Select(x => (byte)x).ToArray();
+            var byteRange = Enumerable.Range(0, 512 + 64).Select(x => (byte)((x & 0x7f) | 0x01)).ToArray();
             var expectedByteRange = byteRange
                                     .Concat(byteRange)
                                     .Concat(byteRange)
@@ -72,7 +99,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
 
                 for (var i = 0; i < expectedByteRange.Length; i++)
                 {
-                    var sb = (byte)s[i];
+                    var sb = (byte)((s[i] & 0x7f) | 0x01);
                     var b = expectedByteRange[i];
 
                     Assert.Equal(sb, b);
@@ -88,7 +115,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
         [Fact]
         private void LargeAllocationProducesCorrectResults()
         {
-            var byteRange = Enumerable.Range(0, 16384 + 64).Select(x => (byte)x).ToArray();
+            var byteRange = Enumerable.Range(0, 16384 + 64).Select(x => (byte)((x & 0x7f) | 0x01)).ToArray();
             var expectedByteRange = byteRange.Concat(byteRange).ToArray();
             using (var pool = new MemoryPool())
             {
@@ -113,7 +140,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
 
                 for (var i = 0; i < expectedByteRange.Length; i++)
                 {
-                    var sb = (byte)s[i];
+                    var sb = (byte)((s[i] & 0x7f) | 0x01);
                     var b = expectedByteRange[i];
 
                     Assert.Equal(sb, b);