Fix for CRLF injection vulnerability (copy commit 483b1d8d from main)

Refit.Tests/AuthenticatedClientHandlerTests.cs

@@ -23,6 +23,12 @@ public interface IMyAuthenticatedService
             Task<string> GetAuthenticated();
         }
 
+        public interface IInheritedAuthenticatedServiceWithHeadersCRLF //: IAuthenticatedServiceWithHeaders
+        {
+            [Get("/get-inherited-thing\r\n\r\nGET /smuggled")]
+            Task<string> 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<ArgumentException>(async () =>
+            {
+                var fixture = RestService.For<IInheritedAuthenticatedServiceWithHeadersCRLF>(
+                        "http://api",
+                        settings
+                    );
+
+                var result = await fixture.GetInheritedThing();
+            });
+        }
     }
 }

Refit.Tests/RefitStubs.Net46.cs

@@ -1793,6 +1793,46 @@ Task<ApiResponse<HttpContent>> 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;
+
+    /// <inheritdoc />
+    [global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage]
+    [global::System.Diagnostics.DebuggerNonUserCode]
+    [Preserve]
+    [global::System.Reflection.Obfuscation(Exclude=true)]
+    partial class AutoGeneratedAuthenticatedClientHandlerTestsIInheritedAuthenticatedServiceWithHeadersCRLF : AuthenticatedClientHandlerTests.IInheritedAuthenticatedServiceWithHeadersCRLF
+    {
+        /// <inheritdoc />
+        public HttpClient Client { get; protected set; }
+        readonly IRequestBuilder requestBuilder;
+
+        /// <inheritdoc />
+        public AutoGeneratedAuthenticatedClientHandlerTestsIInheritedAuthenticatedServiceWithHeadersCRLF(HttpClient client, IRequestBuilder requestBuilder)
+        {
+            Client = client;
+            this.requestBuilder = requestBuilder;
+        }
+
+        /// <inheritdoc />
+        Task<string> AuthenticatedClientHandlerTests.IInheritedAuthenticatedServiceWithHeadersCRLF.GetInheritedThing()
+        {
+            var arguments = new object[] {  };
+            var func = requestBuilder.BuildRestResultFuncForMethod("GetInheritedThing", new Type[] {  });
+            return (Task<string>)func(Client, arguments);
+        }
+    }
+}
+
 namespace Refit.Tests
 {
     using global::System;

Refit.Tests/RefitStubs.NetCore2.cs

@@ -1793,6 +1793,46 @@ Task<ApiResponse<HttpContent>> 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;
+
+    /// <inheritdoc />
+    [global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage]
+    [global::System.Diagnostics.DebuggerNonUserCode]
+    [Preserve]
+    [global::System.Reflection.Obfuscation(Exclude=true)]
+    partial class AutoGeneratedAuthenticatedClientHandlerTestsIInheritedAuthenticatedServiceWithHeadersCRLF : AuthenticatedClientHandlerTests.IInheritedAuthenticatedServiceWithHeadersCRLF
+    {
+        /// <inheritdoc />
+        public HttpClient Client { get; protected set; }
+        readonly IRequestBuilder requestBuilder;
+
+        /// <inheritdoc />
+        public AutoGeneratedAuthenticatedClientHandlerTestsIInheritedAuthenticatedServiceWithHeadersCRLF(HttpClient client, IRequestBuilder requestBuilder)
+        {
+            Client = client;
+            this.requestBuilder = requestBuilder;
+        }
+
+        /// <inheritdoc />
+        Task<string> AuthenticatedClientHandlerTests.IInheritedAuthenticatedServiceWithHeadersCRLF.GetInheritedThing()
+        {
+            var arguments = new object[] {  };
+            var func = requestBuilder.BuildRestResultFuncForMethod("GetInheritedThing", new Type[] {  });
+            return (Task<string>)func(Client, arguments);
+        }
+    }
+}
+
 namespace Refit.Tests
 {
     using global::System;

Refit.Tests/RefitStubs.NetCore3.cs

@@ -1793,6 +1793,46 @@ Task<ApiResponse<HttpContent>> 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;
+
+    /// <inheritdoc />
+    [global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage]
+    [global::System.Diagnostics.DebuggerNonUserCode]
+    [Preserve]
+    [global::System.Reflection.Obfuscation(Exclude=true)]
+    partial class AutoGeneratedAuthenticatedClientHandlerTestsIInheritedAuthenticatedServiceWithHeadersCRLF : AuthenticatedClientHandlerTests.IInheritedAuthenticatedServiceWithHeadersCRLF
+    {
+        /// <inheritdoc />
+        public HttpClient Client { get; protected set; }
+        readonly IRequestBuilder requestBuilder;
+
+        /// <inheritdoc />
+        public AutoGeneratedAuthenticatedClientHandlerTestsIInheritedAuthenticatedServiceWithHeadersCRLF(HttpClient client, IRequestBuilder requestBuilder)
+        {
+            Client = client;
+            this.requestBuilder = requestBuilder;
+        }
+
+        /// <inheritdoc />
+        Task<string> AuthenticatedClientHandlerTests.IInheritedAuthenticatedServiceWithHeadersCRLF.GetInheritedThing()
+        {
+            var arguments = new object[] {  };
+            var func = requestBuilder.BuildRestResultFuncForMethod("GetInheritedThing", new Type[] {  });
+            return (Task<string>)func(Client, arguments);
+        }
+    }
+}
+
 namespace Refit.Tests
 {
     using global::System;

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
+        }
     }
 }

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<int, RestMethodParameterInfo> BuildParameterMap(string relativePath, List<ParameterInfo> parameterInfo)