Skip to content

Commit

Permalink
[Headers][Fix] Now filtering out CRLF injection from headers
Browse files Browse the repository at this point in the history
  • Loading branch information
JeremyBP committed Dec 9, 2024
1 parent b615b1b commit 7b05103
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand Down
10 changes: 10 additions & 0 deletions Apizr/Src/Apizr/ApizrBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -372,13 +372,23 @@ 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);
}
else if (builder.ApizrOptions.BasePathFactory != null)
{
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);
}
Expand Down
18 changes: 14 additions & 4 deletions Apizr/Src/Apizr/Extending/HttpRequestMessageExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
Expand All @@ -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
}
}
}

0 comments on commit 7b05103

Please sign in to comment.