diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/Resources/Strings.resx b/src/libraries/System.Net.Http.WinHttpHandler/src/Resources/Strings.resx index 82e7445d67e742..5a6dd27e582283 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/Resources/Strings.resx @@ -135,4 +135,10 @@ Request version value must be one of 1.0, 1.1, 2.0, or 3.0. + + Request headers must contain only ASCII characters. + + + Request headers must not contain CR, LF, or NUL characters. + diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs index 00617aee48378e..09d04cbe59b70f 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs @@ -709,6 +709,34 @@ private static WinHttpChunkMode GetChunkedModeForSend(HttpRequestMessage request return chunkedMode; } + private static bool IsValidHeaderChar(char c) + { + // Allow Latin-1 characters (0-255) but reject dangerous control characters + return c <= 255 && c != '\0' && c != '\r' && c != '\n'; + } + + private static void ValidateHeaderValues(System.Net.Http.Headers.HttpHeaders headers) + { + foreach (var header in headers) + { + foreach (var value in header.Value) + { + ValidateHeaderValue(value); + } + } + } + + private static void ValidateHeaderValue(string value) + { + for (int i = 0; i < value.Length; i++) + { + if (!IsValidHeaderChar(value[i])) + { + throw new HttpRequestException(SR.net_http_headers_invalid_chars); + } + } + } + private static void AddRequestHeaders( SafeWinHttpHandle requestHandle, HttpRequestMessage requestMessage, @@ -734,14 +762,17 @@ private static void AddRequestHeaders( string? cookieHeader = WinHttpCookieContainerAdapter.GetCookieHeader(requestMessage.RequestUri, cookies); if (!string.IsNullOrEmpty(cookieHeader)) { + ValidateHeaderValue(cookieHeader); requestHeadersBuffer.AppendLine(cookieHeader); } } - // Serialize general request headers. - requestHeadersBuffer.AppendLine(requestMessage.Headers.ToString()); + // Validate and serialize general request headers + ValidateHeaderValues(requestMessage.Headers); + string generalHeaders = requestMessage.Headers.ToString(); + requestHeadersBuffer.Append(generalHeaders); - // Serialize entity-body (content) headers. + // Serialize entity-body (content) headers and validate for ASCII. if (requestMessage.Content != null) { // TODO https://github.com/dotnet/runtime/issues/16162: @@ -754,7 +785,10 @@ private static void AddRequestHeaders( requestMessage.Content.Headers.ContentLength = contentLength; } - requestHeadersBuffer.AppendLine(requestMessage.Content.Headers.ToString()); + // Validate and serialize entity-body (content) headers + ValidateHeaderValues(requestMessage.Content.Headers); + string contentHeaders = requestMessage.Content.Headers.ToString(); + requestHeadersBuffer.Append(contentHeaders); } // Add request headers to WinHTTP request handle. diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpHandlerTest.cs b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpHandlerTest.cs index 15f7accca5d46f..91220735e6bf07 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpHandlerTest.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpHandlerTest.cs @@ -837,6 +837,107 @@ public void SendAsync_MultipleCallsWithDispose_NoHandleLeaksManuallyVerifiedUsin } } + [Fact] + public void SendAsync_RequestWithDangerousControlHeaderValue_ThrowsHttpRequestException() + { + using (var handler = new WinHttpHandler()) + { + TestServer.SetResponse(DecompressionMethods.None, TestServer.ExpectedResponseBody); + + var invoker = new HttpMessageInvoker(handler, false); + var request = new HttpRequestMessage(HttpMethod.Get, TestServer.FakeServerEndpoint); + request.Headers.Add("Custom-Header", "HeaderValue\0WithNUL"); + + var ex = Assert.Throws(() => + { + Task task = invoker.SendAsync(request, CancellationToken.None); + task.GetAwaiter().GetResult(); + }); + + Assert.Contains("CR, LF, or NUL", ex.Message); + } + } + + [Fact] + public void SendAsync_RequestWithLatin1HeaderValue_Succeeds() + { + using (var handler = new WinHttpHandler()) + { + TestServer.SetResponse(DecompressionMethods.None, TestServer.ExpectedResponseBody); + + var invoker = new HttpMessageInvoker(handler, false); + var request = new HttpRequestMessage(HttpMethod.Get, TestServer.FakeServerEndpoint); + request.Headers.Add("Custom-Header", "HeaderValue\u00A9WithLatin1"); + + Task task = invoker.SendAsync(request, CancellationToken.None); + using (HttpResponseMessage response = task.GetAwaiter().GetResult()) + { + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + } + } + + [Fact] + public void SendAsync_RequestWithAsciiHeaderValue_Succeeds() + { + using (var handler = new WinHttpHandler()) + { + TestServer.SetResponse(DecompressionMethods.None, TestServer.ExpectedResponseBody); + + var invoker = new HttpMessageInvoker(handler, false); + var request = new HttpRequestMessage(HttpMethod.Get, TestServer.FakeServerEndpoint); + request.Headers.Add("Custom-Header", "ValidASCIIHeaderValue123"); + + Task task = invoker.SendAsync(request, CancellationToken.None); + using (HttpResponseMessage response = task.GetAwaiter().GetResult()) + { + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + } + } + + [Fact] + public void SendAsync_RequestWithDangerousControlContentHeader_ThrowsHttpRequestException() + { + using (var handler = new WinHttpHandler()) + { + TestServer.SetResponse(DecompressionMethods.None, TestServer.ExpectedResponseBody); + + var invoker = new HttpMessageInvoker(handler, false); + var request = new HttpRequestMessage(HttpMethod.Post, TestServer.FakeServerEndpoint); + request.Content = new StringContent("test content"); + request.Content.Headers.Add("Custom-Content-Header", "ContentValue\0WithNUL"); + + var ex = Assert.Throws(() => + { + Task task = invoker.SendAsync(request, CancellationToken.None); + task.GetAwaiter().GetResult(); + }); + + Assert.Contains("CR, LF, or NUL", ex.Message); + } + } + + [Fact] + public void SendAsync_RequestWithLatin1ContentHeader_Succeeds() + { + using (var handler = new WinHttpHandler()) + { + TestServer.SetResponse(DecompressionMethods.None, TestServer.ExpectedResponseBody); + + var invoker = new HttpMessageInvoker(handler, false); + var request = new HttpRequestMessage(HttpMethod.Post, TestServer.FakeServerEndpoint); + request.Content = new StringContent("test content"); + request.Content.Headers.Add("Custom-Content-Header", "ContentValue\u00A9WithLatin1"); + + Task task = invoker.SendAsync(request, CancellationToken.None); + using (HttpResponseMessage response = task.GetAwaiter().GetResult()) + { + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + } + } + // Commented out as the test relies on finalizer for cleanup and only has value as written // when run on its own and manual analysis is done of logs. //[Fact]