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..ac1f180faeab00 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,7 @@ Request version value must be one of 1.0, 1.1, 2.0, or 3.0. + + {0} headers must be valid Latin-1 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..6c17a3f354ab88 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,20 @@ private static WinHttpChunkMode GetChunkedModeForSend(HttpRequestMessage request return chunkedMode; } + private static bool ContainsOnlyValidLatin1(string headerString) + { + foreach (char c in headerString) + { + // See https://www.rfc-editor.org/rfc/rfc9110.html#section-5.5-5: + // "Field values containing CR, LF, or NUL characters are invalid and dangerous" + if (c <= 0 || c > 255 || c == '\r' || c == '\n') + { + return false; + } + } + return true; + } + private static void AddRequestHeaders( SafeWinHttpHandle requestHandle, HttpRequestMessage requestMessage, @@ -739,7 +753,12 @@ private static void AddRequestHeaders( } // Serialize general request headers. - requestHeadersBuffer.AppendLine(requestMessage.Headers.ToString()); + string requestHeaders = requestMessage.Headers.ToString(); + if (!ContainsOnlyValidLatin1(requestHeaders)) + { + throw new FormatException(SR.Format(SR.net_http_invalid_header_value, nameof(HttpRequestMessage))); + } + requestHeadersBuffer.AppendLine(requestHeaders); // Serialize entity-body (content) headers. if (requestMessage.Content != null) @@ -754,7 +773,12 @@ private static void AddRequestHeaders( requestMessage.Content.Headers.ContentLength = contentLength; } - requestHeadersBuffer.AppendLine(requestMessage.Content.Headers.ToString()); + string contentHeaders = requestMessage.Content.Headers.ToString(); + if (!ContainsOnlyValidLatin1(contentHeaders)) + { + throw new FormatException(SR.Format(SR.net_http_invalid_header_value, nameof(HttpContent))); + } + requestHeadersBuffer.AppendLine(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..739f55eb87e74a 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpHandlerTest.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpHandlerTest.cs @@ -121,7 +121,7 @@ public void TcpKeepalive_WhenDisabled_DoesntSetOptions() SendRequestHelper.Send( handler, - () => handler.TcpKeepAliveEnabled = false ); + () => handler.TcpKeepAliveEnabled = false); Assert.Null(APICallHistory.WinHttpOptionTcpKeepAlive); } @@ -837,6 +837,94 @@ public void SendAsync_MultipleCallsWithDispose_NoHandleLeaksManuallyVerifiedUsin } } + [Theory] + [InlineData('\r', HeaderType.Request)] + [InlineData('\n', HeaderType.Request)] + [InlineData('\0', HeaderType.Request)] + [InlineData('\r', HeaderType.Content)] + [InlineData('\n', HeaderType.Content)] + [InlineData('\0', HeaderType.Content)] + [InlineData('\r', HeaderType.Cookie)] + [InlineData('\n', HeaderType.Cookie)] + [InlineData('\0', HeaderType.Cookie)] + public async Task SendAsync_RequestWithDangerousControlHeaderValue_ThrowsHttpRequestException(char dangerousChar, HeaderType headerType) + { + var handler = new WinHttpHandler(); + using (var client = new HttpClient(handler)) + { + TestServer.SetResponse(DecompressionMethods.None, TestServer.ExpectedResponseBody); + + var request = new HttpRequestMessage(HttpMethod.Get, TestServer.FakeServerEndpoint); + switch (headerType) + { + case HeaderType.Request: + request.Headers.Add("Custom-Header", $"HeaderValue{dangerousChar}WithControlChar"); + break; + case HeaderType.Content: + request.Content = new StringContent("test content"); + request.Content.Headers.Add("Custom-Content-Header", $"ContentValue{dangerousChar}WithControlChar"); + break; + case HeaderType.Cookie: + handler.CookieUsePolicy = CookieUsePolicy.UseSpecifiedCookieContainer; + handler.CookieContainer = new CookieContainer(); + handler.CookieContainer.Add(new Uri(TestServer.FakeServerEndpoint), new Cookie("CustomCookie", $"Value{dangerousChar}WithControlChar")); + break; + } + + var ex = await Assert.ThrowsAsync(() => client.SendAsync(request)); + var fex = Assert.IsType(ex.InnerException); + Assert.Contains("Latin-1", fex.Message); + } + } + + [Theory] + [InlineData('\u00A9', HeaderType.Request)] + [InlineData('\u00FF', HeaderType.Request)] + [InlineData('\u0001', HeaderType.Request)] + [InlineData('\u00A9', HeaderType.Content)] + [InlineData('\u00FF', HeaderType.Content)] + [InlineData('\u0001', HeaderType.Content)] + [InlineData('\u00A9', HeaderType.Cookie)] + [InlineData('\u00FF', HeaderType.Cookie)] + [InlineData('\u0001', HeaderType.Cookie)] + public async Task SendAsync_RequestWithLatin1HeaderValue_Succeeds(char safeChar, HeaderType headerType) + { + var handler = new WinHttpHandler(); + using (var client = new HttpClient(handler)) + { + TestServer.SetResponse(DecompressionMethods.None, TestServer.ExpectedResponseBody); + + var request = new HttpRequestMessage(HttpMethod.Get, TestServer.FakeServerEndpoint); + switch (headerType) + { + case HeaderType.Request: + request.Headers.Add("Custom-Header", $"HeaderValue{safeChar}WithSafeChar"); + break; + case HeaderType.Content: + request.Content = new StringContent("test content"); + request.Content.Headers.Add("Custom-Content-Header", $"ContentValue{safeChar}WithSafeChar"); + break; + case HeaderType.Cookie: + handler.CookieUsePolicy = CookieUsePolicy.UseSpecifiedCookieContainer; + handler.CookieContainer = new CookieContainer(); + handler.CookieContainer.Add(new Uri(TestServer.FakeServerEndpoint), new Cookie("CustomCookie", $"Value{safeChar}WithSafeChar")); + break; + } + + using (HttpResponseMessage response = await client.SendAsync(request)) + { + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + } + } + + public enum HeaderType + { + Request, + Content, + Cookie + } + // 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] diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/AuthenticationHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/AuthenticationHeaderValue.cs index 930cb8f8408587..e8291087b406a9 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/AuthenticationHeaderValue.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/AuthenticationHeaderValue.cs @@ -119,7 +119,7 @@ internal static int GetAuthenticationLength(string? input, int startIndex, out o parsedValue = null; - if (string.IsNullOrEmpty(input) || (startIndex >= input.Length) || HttpRuleParser.ContainsNewLine(input, startIndex)) + if (string.IsNullOrEmpty(input) || (startIndex >= input.Length) || HttpRuleParser.ContainsNewLineOrNull(input, startIndex)) { return 0; } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/GenericHeaderParser.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/GenericHeaderParser.cs index 9e93c595f377e3..6706eb10064533 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/GenericHeaderParser.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/GenericHeaderParser.cs @@ -119,7 +119,7 @@ private static int ParseMultipleEntityTags(string value, int startIndex, out obj /// private static int ParseWithoutValidation(string value, int startIndex, out object? parsedValue) { - if (HttpRuleParser.ContainsNewLine(value, startIndex)) + if (HttpRuleParser.ContainsNewLineOrNull(value, startIndex)) { parsedValue = null; return 0; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs index ab0df44d9d566d..76f3f926243180 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs @@ -806,7 +806,7 @@ private static void ParseSingleRawHeaderValue(HeaderStoreItemInfo info, HeaderDe Debug.Assert(Monitor.IsEntered(info)); if (descriptor.Parser == null) { - if (HttpRuleParser.ContainsNewLine(rawValue)) + if (HttpRuleParser.ContainsNewLineOrNull(rawValue)) { if (NetEventSource.Log.IsEnabled()) NetEventSource.Error(null, SR.Format(SR.net_http_log_headers_no_newlines, descriptor.Name, rawValue)); AddInvalidValue(info, rawValue); @@ -1024,7 +1024,7 @@ private static void ParseAndAddValue(HeaderDescriptor descriptor, HeaderStoreIte if (descriptor.Parser == null) { // If we don't have a parser for the header, we consider the value valid if it doesn't contains - // newline characters. We add the values as "parsed value". Note that we allow empty values. + // newline or \0 characters. We add the values as "parsed value". Note that we allow empty values. CheckContainsNewLine(value); AddParsedValue(info, value ?? string.Empty); return; @@ -1134,7 +1134,7 @@ internal static void CheckContainsNewLine(string? value) return; } - if (HttpRuleParser.ContainsNewLine(value)) + if (HttpRuleParser.ContainsNewLineOrNull(value)) { throw new FormatException(SR.net_http_headers_no_newlines); } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueHeaderValue.cs index 6920a881828fcc..694fdd287bee4d 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueHeaderValue.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueHeaderValue.cs @@ -371,7 +371,7 @@ private static void CheckValueFormat(string? value) ThrowFormatException(value); } } - else if (HttpRuleParser.ContainsNewLine(value)) + else if (HttpRuleParser.ContainsNewLineOrNull(value)) { ThrowFormatException(value); } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs index a24dc5d457d040..9957aa41d62089 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs @@ -90,7 +90,7 @@ public string? ReasonPhrase } set { - if ((value != null) && HttpRuleParser.ContainsNewLine(value)) + if ((value != null) && HttpRuleParser.ContainsNewLineOrNull(value)) { throw new FormatException(SR.net_http_reasonphrase_format_error); } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRuleParser.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRuleParser.cs index cad5eda382ca00..81a087dcafdbb6 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRuleParser.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRuleParser.cs @@ -82,8 +82,10 @@ internal static int GetWhitespaceLength(string input, int startIndex) return input.Length - startIndex; } - internal static bool ContainsNewLine(string value, int startIndex = 0) => - value.AsSpan(startIndex).ContainsAny('\r', '\n'); + // See https://www.rfc-editor.org/rfc/rfc9110.html#section-5.5-5: + // "Field values containing CR, LF, or NUL characters are invalid and dangerous" + internal static bool ContainsNewLineOrNull(string value, int startIndex = 0) => + value.AsSpan(startIndex).ContainsAny('\r', '\n', '\0'); internal static int GetNumberLength(string input, int startIndex, bool allowDecimal) {