diff --git a/Refit.Tests/AuthenticatedClientHandlerTests.cs b/Refit.Tests/AuthenticatedClientHandlerTests.cs index 5806fa27c..d51e14bd4 100644 --- a/Refit.Tests/AuthenticatedClientHandlerTests.cs +++ b/Refit.Tests/AuthenticatedClientHandlerTests.cs @@ -23,6 +23,12 @@ public interface IMyAuthenticatedService Task GetAuthenticated(); } + public interface IInheritedAuthenticatedServiceWithHeadersCRLF //: IAuthenticatedServiceWithHeaders + { + [Get("/get-inherited-thing\r\n\r\nGET /smuggled")] + Task GetInheritedThing(); + } + [Fact] public void DefaultHandlerIsHttpClientHandler() @@ -120,5 +126,31 @@ public async void AuthenticatedHandlerWithParamUsesAuth() Assert.Equal("Ok", result); } + + [Fact] + public async void AuthentictedMethodFromInheritedClassWithHeadersAttributeUsesAuth_WithCRLFCheck() + { + var handler = new MockHttpMessageHandler(); + var settings = new RefitSettings() + { + AuthorizationHeaderValueGetter = () => Task.FromResult("tokenValue"), + HttpMessageHandlerFactory = () => handler, + }; + + handler + .Expect(HttpMethod.Get, "http://api/get-inherited-thing") + .WithHeaders("Authorization", "Bearer tokenValue") + .Respond("text/plain", "Ok"); + + await Assert.ThrowsAsync(async () => + { + var fixture = RestService.For( + "http://api", + settings + ); + + var result = await fixture.GetInheritedThing(); + }); + } } } diff --git a/Refit.Tests/RefitStubs.Net46.cs b/Refit.Tests/RefitStubs.Net46.cs index ca4722c3a..44e5bc0fe 100644 --- a/Refit.Tests/RefitStubs.Net46.cs +++ b/Refit.Tests/RefitStubs.Net46.cs @@ -1793,6 +1793,46 @@ Task> IHttpContentApi.PostFileUploadWithMetadata(HttpCo } } +namespace Refit.Tests +{ + using global::System; + using global::System.Collections.Generic; + using global::System.Net; + using global::System.Net.Http; + using global::System.Text; + using global::System.Threading.Tasks; + using global::RichardSzalay.MockHttp; + using global::Refit; + using global::Xunit; + + /// + [global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] + [global::System.Diagnostics.DebuggerNonUserCode] + [Preserve] + [global::System.Reflection.Obfuscation(Exclude=true)] + partial class AutoGeneratedAuthenticatedClientHandlerTestsIInheritedAuthenticatedServiceWithHeadersCRLF : AuthenticatedClientHandlerTests.IInheritedAuthenticatedServiceWithHeadersCRLF + { + /// + public HttpClient Client { get; protected set; } + readonly IRequestBuilder requestBuilder; + + /// + public AutoGeneratedAuthenticatedClientHandlerTestsIInheritedAuthenticatedServiceWithHeadersCRLF(HttpClient client, IRequestBuilder requestBuilder) + { + Client = client; + this.requestBuilder = requestBuilder; + } + + /// + Task AuthenticatedClientHandlerTests.IInheritedAuthenticatedServiceWithHeadersCRLF.GetInheritedThing() + { + var arguments = new object[] { }; + var func = requestBuilder.BuildRestResultFuncForMethod("GetInheritedThing", new Type[] { }); + return (Task)func(Client, arguments); + } + } +} + namespace Refit.Tests { using global::System; diff --git a/Refit.Tests/RefitStubs.NetCore2.cs b/Refit.Tests/RefitStubs.NetCore2.cs index ca4722c3a..44e5bc0fe 100644 --- a/Refit.Tests/RefitStubs.NetCore2.cs +++ b/Refit.Tests/RefitStubs.NetCore2.cs @@ -1793,6 +1793,46 @@ Task> IHttpContentApi.PostFileUploadWithMetadata(HttpCo } } +namespace Refit.Tests +{ + using global::System; + using global::System.Collections.Generic; + using global::System.Net; + using global::System.Net.Http; + using global::System.Text; + using global::System.Threading.Tasks; + using global::RichardSzalay.MockHttp; + using global::Refit; + using global::Xunit; + + /// + [global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] + [global::System.Diagnostics.DebuggerNonUserCode] + [Preserve] + [global::System.Reflection.Obfuscation(Exclude=true)] + partial class AutoGeneratedAuthenticatedClientHandlerTestsIInheritedAuthenticatedServiceWithHeadersCRLF : AuthenticatedClientHandlerTests.IInheritedAuthenticatedServiceWithHeadersCRLF + { + /// + public HttpClient Client { get; protected set; } + readonly IRequestBuilder requestBuilder; + + /// + public AutoGeneratedAuthenticatedClientHandlerTestsIInheritedAuthenticatedServiceWithHeadersCRLF(HttpClient client, IRequestBuilder requestBuilder) + { + Client = client; + this.requestBuilder = requestBuilder; + } + + /// + Task AuthenticatedClientHandlerTests.IInheritedAuthenticatedServiceWithHeadersCRLF.GetInheritedThing() + { + var arguments = new object[] { }; + var func = requestBuilder.BuildRestResultFuncForMethod("GetInheritedThing", new Type[] { }); + return (Task)func(Client, arguments); + } + } +} + namespace Refit.Tests { using global::System; diff --git a/Refit.Tests/RefitStubs.NetCore3.cs b/Refit.Tests/RefitStubs.NetCore3.cs index ca4722c3a..44e5bc0fe 100644 --- a/Refit.Tests/RefitStubs.NetCore3.cs +++ b/Refit.Tests/RefitStubs.NetCore3.cs @@ -1793,6 +1793,46 @@ Task> IHttpContentApi.PostFileUploadWithMetadata(HttpCo } } +namespace Refit.Tests +{ + using global::System; + using global::System.Collections.Generic; + using global::System.Net; + using global::System.Net.Http; + using global::System.Text; + using global::System.Threading.Tasks; + using global::RichardSzalay.MockHttp; + using global::Refit; + using global::Xunit; + + /// + [global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] + [global::System.Diagnostics.DebuggerNonUserCode] + [Preserve] + [global::System.Reflection.Obfuscation(Exclude=true)] + partial class AutoGeneratedAuthenticatedClientHandlerTestsIInheritedAuthenticatedServiceWithHeadersCRLF : AuthenticatedClientHandlerTests.IInheritedAuthenticatedServiceWithHeadersCRLF + { + /// + public HttpClient Client { get; protected set; } + readonly IRequestBuilder requestBuilder; + + /// + public AutoGeneratedAuthenticatedClientHandlerTestsIInheritedAuthenticatedServiceWithHeadersCRLF(HttpClient client, IRequestBuilder requestBuilder) + { + Client = client; + this.requestBuilder = requestBuilder; + } + + /// + Task AuthenticatedClientHandlerTests.IInheritedAuthenticatedServiceWithHeadersCRLF.GetInheritedThing() + { + var arguments = new object[] { }; + var func = requestBuilder.BuildRestResultFuncForMethod("GetInheritedThing", new Type[] { }); + return (Task)func(Client, arguments); + } + } +} + namespace Refit.Tests { using global::System; diff --git a/Refit/RequestBuilderImplementation.cs b/Refit/RequestBuilderImplementation.cs index 29bdf9742..b9c121f77 100644 --- a/Refit/RequestBuilderImplementation.cs +++ b/Refit/RequestBuilderImplementation.cs @@ -853,6 +853,10 @@ static void SetHeader(HttpRequestMessage request, string name, string value) if (value == null) return; + // CRLF injection protection + name = EnsureSafe(name); + value = EnsureSafe(value); + var added = request.Headers.TryAddWithoutValidation(name, value); // Don't even bother trying to add the header as a content header @@ -862,5 +866,13 @@ static void SetHeader(HttpRequestMessage request, string name, string value) request.Content.Headers.TryAddWithoutValidation(name, value); } } + + 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 + } } } diff --git a/Refit/RestMethodInfo.cs b/Refit/RestMethodInfo.cs index 2bf5710f1..da156f161 100644 --- a/Refit/RestMethodInfo.cs +++ b/Refit/RestMethodInfo.cs @@ -134,6 +134,12 @@ void VerifyUrlPathIsSane(string relativePath) if (!relativePath.StartsWith("/")) throw new ArgumentException($"URL path {relativePath} must start with '/' and be of the form '/foo/bar/baz'"); + + // CRLF injection protection + if (relativePath.Contains("\r") || relativePath.Contains("\n")) + throw new ArgumentException( + $"URL path {relativePath} must not contain CR or LF characters" + ); } Dictionary BuildParameterMap(string relativePath, List parameterInfo)