From 7b051036db3dfc59f042a3a90bde0d82cb2f9a5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20BRUN-PICARD?= Date: Mon, 9 Dec 2024 10:56:20 +0100 Subject: [PATCH] [Headers][Fix] Now filtering out CRLF injection from headers --- .../ServiceCollectionExtensions.cs | 5 +++++ Apizr/Src/Apizr/ApizrBuilder.cs | 10 ++++++++++ .../Extending/HttpRequestMessageExtensions.cs | 18 ++++++++++++++---- 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/Apizr/Src/Apizr.Extensions.Microsoft.DependencyInjection/ServiceCollectionExtensions.cs b/Apizr/Src/Apizr.Extensions.Microsoft.DependencyInjection/ServiceCollectionExtensions.cs index e13336a6..d7503521 100644 --- a/Apizr/Src/Apizr.Extensions.Microsoft.DependencyInjection/ServiceCollectionExtensions.cs +++ b/Apizr/Src/Apizr.Extensions.Microsoft.DependencyInjection/ServiceCollectionExtensions.cs @@ -528,6 +528,11 @@ internal static Type AddApizrManagerFor( managerOptions.BaseUriFactory?.Invoke(serviceProvider); managerOptions.BaseAddressFactory?.Invoke(serviceProvider); managerOptions.BasePathFactory?.Invoke(serviceProvider); + + if (managerOptions.BasePath?.Contains("\r") == true || + managerOptions.BasePath?.Contains("\n") == true) + throw new ArgumentException($"URL path {managerOptions.BasePath} must not contain CR or LF characters"); + var baseAddress = managerOptions.BaseUri?.ToString() ?? managerOptions.BaseAddress; if (Uri.TryCreate(UrlHelper.Combine(baseAddress, managerOptions.BasePath), UriKind.RelativeOrAbsolute, out var baseUri)) { diff --git a/Apizr/Src/Apizr/ApizrBuilder.cs b/Apizr/Src/Apizr/ApizrBuilder.cs index 3975758e..4adf9562 100644 --- a/Apizr/Src/Apizr/ApizrBuilder.cs +++ b/Apizr/Src/Apizr/ApizrBuilder.cs @@ -372,6 +372,11 @@ private static IApizrManagerOptions CreateManagerOptions(IApizrCommonOptions com { builder.ApizrOptions.BaseAddressFactory?.Invoke(); builder.ApizrOptions.BasePathFactory?.Invoke(); + + if(builder.ApizrOptions.BasePath?.Contains("\r") == true || + builder.ApizrOptions.BasePath?.Contains("\n") == true) + throw new ArgumentException($"URL path {builder.ApizrOptions.BasePath} must not contain CR or LF characters"); + if (Uri.TryCreate(UrlHelper.Combine(builder.ApizrOptions.BaseAddress, builder.ApizrOptions.BasePath), UriKind.RelativeOrAbsolute, out var baseUri)) builder.WithBaseAddress(baseUri); } @@ -379,6 +384,11 @@ private static IApizrManagerOptions CreateManagerOptions(IApizrCommonOptions com { builder.ApizrOptions.BaseUriFactory?.Invoke(); builder.ApizrOptions.BasePathFactory?.Invoke(); + + if (builder.ApizrOptions.BasePath?.Contains("\r") == true || + builder.ApizrOptions.BasePath?.Contains("\n") == true) + throw new ArgumentException($"URL path {builder.ApizrOptions.BasePath} must not contain CR or LF characters"); + if (Uri.TryCreate(UrlHelper.Combine(builder.ApizrOptions.BaseUri?.ToString(), builder.ApizrOptions.BasePath), UriKind.RelativeOrAbsolute, out var baseUri)) builder.WithBaseAddress(baseUri); } diff --git a/Apizr/Src/Apizr/Extending/HttpRequestMessageExtensions.cs b/Apizr/Src/Apizr/Extending/HttpRequestMessageExtensions.cs index c3ea5923..6a4b2466 100644 --- a/Apizr/Src/Apizr/Extending/HttpRequestMessageExtensions.cs +++ b/Apizr/Src/Apizr/Extending/HttpRequestMessageExtensions.cs @@ -176,8 +176,9 @@ internal static bool TrySetHeader(this HttpHeaders headers, string key, string v if (value == null || removeOnly) return true; - // Remove redact stars (*) if any - var headerValue = value.StartsWith("*") && value.EndsWith("*") ? value.Trim('*') : value; + // Remove redact stars (*) if any & CRLF injection protection + key = EnsureSafe(key); + var headerValue = EnsureSafe(value.StartsWith("*") && value.EndsWith("*") ? value.Trim('*') : value); return headers.TryAddWithoutValidation(key, headerValue); } @@ -202,9 +203,10 @@ internal static bool TrySetHeader(this HttpHeaders headers, string key, IEnumera if (values == null || removeOnly) return true; - // Remove redact stars (*) if any + // Remove redact stars (*) if any & CRLF injection protection + key = EnsureSafe(key); var headerValues = values.Select(headerValue => - headerValue.StartsWith("*") && headerValue.EndsWith("*") ? headerValue.Trim('*') : headerValue) + EnsureSafe(headerValue.StartsWith("*") && headerValue.EndsWith("*") ? headerValue.Trim('*') : headerValue)) .ToList(); return headers.TryAddWithoutValidation(key, headerValues); @@ -226,5 +228,13 @@ internal static bool TryGetHeaderKeyValue(string header, out string key, out str return !string.IsNullOrWhiteSpace(key); } + + internal static string EnsureSafe(string value) + { + // Remove CR and LF characters +#pragma warning disable CA1307 // Specify StringComparison for clarity + return value.Replace("\r", string.Empty).Replace("\n", string.Empty); +#pragma warning restore CA1307 // Specify StringComparison for clarity + } } }