From 0753f06c284dc0635484bb27a1a76858af9eeb27 Mon Sep 17 00:00:00 2001 From: moozzyk Date: Fri, 27 May 2016 10:32:51 -0700 Subject: [PATCH] Aborting request if a string can't be converted to ASCII --- .../Http/Frame.cs | 11 +++-- .../Http/FrameOfT.cs | 9 ++++ .../Infrastructure/AsciiUtilities.cs | 22 +++++++++- .../MemoryPoolIteratorExtensions.cs | 11 ++--- .../{AsciiDecoder.cs => AsciiDecoding.cs} | 43 +++++++++++++++---- 5 files changed, 79 insertions(+), 17 deletions(-) rename test/Microsoft.AspNetCore.Server.KestrelTests/{AsciiDecoder.cs => AsciiDecoding.cs} (75%) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Http/Frame.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Http/Frame.cs index 7868964958..dbf7463dfe 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Http/Frame.cs +++ b/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, 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) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Http/FrameOfT.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Http/FrameOfT.cs index c0181c5463..13a04dea49 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Http/FrameOfT.cs +++ b/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"); diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Infrastructure/AsciiUtilities.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Infrastructure/AsciiUtilities.cs index e75c547864..69d6fda2aa 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Infrastructure/AsciiUtilities.cs +++ b/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; } } } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Infrastructure/MemoryPoolIteratorExtensions.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Infrastructure/MemoryPoolIteratorExtensions.cs index 315b332fb2..5d1953166e 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Infrastructure/MemoryPoolIteratorExtensions.cs +++ b/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; } diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/AsciiDecoder.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/AsciiDecoding.cs similarity index 75% rename from test/Microsoft.AspNetCore.Server.KestrelTests/AsciiDecoder.cs rename to test/Microsoft.AspNetCore.Server.KestrelTests/AsciiDecoding.cs index 634efc65db..8d62e034c5 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/AsciiDecoder.cs +++ b/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(() => 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);