diff --git a/src/Refitter.Core/Generation/InterfaceGenerator.cs b/src/Refitter.Core/Generation/InterfaceGenerator.cs index 30c95a5de..d772b5226 100644 --- a/src/Refitter.Core/Generation/InterfaceGenerator.cs +++ b/src/Refitter.Core/Generation/InterfaceGenerator.cs @@ -237,7 +237,7 @@ private IEnumerable GenerateMultipleInterface( code.AppendLine($"{Separator}{Separator}{attribute}"); } - code.AppendLine($"{Separator}{Separator}[{verb}(\"{op.Path}\")]") + code.AppendLine($"{Separator}{Separator}[{verb}(\"{ParameterShared.EscapeString(op.Path)}\")]") .AppendLine($"{Separator}{Separator}{returnType} {methodName}({parametersString});") .AppendLine(); @@ -263,7 +263,7 @@ private IEnumerable GenerateMultipleInterface( ", ", parameters.Where(parameter => !parameter.Contains("?"))); - code.AppendLine($"{Separator}{Separator}[{verb}(\"{op.Path}\")]") + code.AppendLine($"{Separator}{Separator}[{verb}(\"{ParameterShared.EscapeString(op.Path)}\")]") .AppendLine($"{Separator}{Separator}{returnType} {methodName}({nonOptionalParametersString});") .AppendLine(); } diff --git a/src/Refitter.Core/Generation/MethodAttributeGenerator.cs b/src/Refitter.Core/Generation/MethodAttributeGenerator.cs index 76094ae7f..0eb949a0d 100644 --- a/src/Refitter.Core/Generation/MethodAttributeGenerator.cs +++ b/src/Refitter.Core/Generation/MethodAttributeGenerator.cs @@ -41,7 +41,7 @@ public string[] Generate(OpenApiOperation operation, CSharpOperationModel operat if (uniqueContentTypes.Any()) { - headers.Add($"\"Accept: {string.Join(", ", uniqueContentTypes)}\""); + headers.Add($"\"Accept: {string.Join(", ", uniqueContentTypes.Select(ParameterShared.EscapeString))}\""); } } @@ -54,7 +54,7 @@ public string[] Generate(OpenApiOperation operation, CSharpOperationModel operat if (!string.IsNullOrWhiteSpace(contentType) && !operationModel.Consumes.Contains("multipart/form-data")) { - headers.Add($"\"Content-Type: {contentType}\""); + headers.Add($"\"Content-Type: {ParameterShared.EscapeString(contentType)}\""); } } diff --git a/src/Refitter.Core/ParameterExtraction/HeaderParameterExtractor.cs b/src/Refitter.Core/ParameterExtraction/HeaderParameterExtractor.cs index 228f87deb..57570fbf4 100644 --- a/src/Refitter.Core/ParameterExtraction/HeaderParameterExtractor.cs +++ b/src/Refitter.Core/ParameterExtraction/HeaderParameterExtractor.cs @@ -27,7 +27,7 @@ public IEnumerable Extract( .Select(p => { var variableName = ParameterShared.GetVariableName(p); - return $"{ParameterShared.JoinAttributes($"Header(\"{p.Name}\")")}{ParameterShared.GetParameterType(p, settings)} {variableName}"; + return $"{ParameterShared.JoinAttributes($"Header(\"{ParameterShared.EscapeString(p.Name)}\")")}{ParameterShared.GetParameterType(p, settings)} {variableName}"; }) .ToList(); } @@ -47,7 +47,7 @@ public IEnumerable Extract( && securityScheme.In == OpenApiSecurityApiKeyLocation.Header && !operationModel.Parameters.Any(p => p.Kind == OpenApiParameterKind.Header && p.IsHeader && p.Name == securityScheme.Name)) { - headerParameters.Add($"[Header(\"{securityScheme.Name}\")] string {ParameterShared.ReplaceUnsafeCharacters(securityScheme.Name)}"); + headerParameters.Add($"[Header(\"{ParameterShared.EscapeString(securityScheme.Name)}\")] string {ParameterShared.ReplaceUnsafeCharacters(securityScheme.Name)}"); } else if (securityScheme is { Type: OpenApiSecuritySchemeType.Http } && string.Equals(securityScheme.Scheme, "bearer", StringComparison.OrdinalIgnoreCase)) diff --git a/src/Refitter.Core/Validation/AttributeStringValidator.cs b/src/Refitter.Core/Validation/AttributeStringValidator.cs new file mode 100644 index 000000000..eeb73f75c --- /dev/null +++ b/src/Refitter.Core/Validation/AttributeStringValidator.cs @@ -0,0 +1,118 @@ +using System.Linq; +using Microsoft.OpenApi; +using Microsoft.OpenApi.Reader; + +namespace Refitter.Core.Validation; + +/// +/// Rejects OpenAPI paths, header names, and content-type keys containing characters that could break +/// out of the generated Refit attribute string literals (e.g. quotes, backslashes, newlines). Generated +/// code escapes these characters, but rejecting them gives a clear error unless validation is skipped. +/// See GHSA-3fhm-p725-h3g3 (path), GHSA-58x9-vjvp-6mx8 (header name), GHSA-p32v-8v8j-j534 (content-type). +/// +internal static class AttributeStringValidator +{ + internal static bool ContainsUnsafeCharacters(string? value) => + value != null && value.Any(c => c is '"' or '\\' || char.IsControl(c)); + + internal static void Validate(OpenApiDocument? document, OpenApiDiagnostic diagnostic) + { + if (document == null) + return; + + // Validate security scheme names for API-key schemes with header location + if (document.Components?.SecuritySchemes != null) + { + foreach (var securityScheme in document.Components.SecuritySchemes) + { + if (securityScheme.Value?.Type == SecuritySchemeType.ApiKey + && securityScheme.Value.In == ParameterLocation.Header + && ContainsUnsafeCharacters(securityScheme.Value.Name)) + { + diagnostic.Errors.Add(new OpenApiError( + securityScheme.Key, + $"Security scheme '{securityScheme.Key}' has header name '{securityScheme.Value.Name}' containing illegal characters and is rejected to prevent code injection into Refit attributes. Use --skip-validation to bypass.")); + } + } + } + + if (document.Paths == null) + return; + + // Accept/Content-Type headers are only emitted from content map keys for OpenAPI 3.0+, + // so only reject unsafe content-type keys for those documents to avoid Swagger 2.0 false positives. + var validateContentTypes = diagnostic.SpecificationVersion != OpenApiSpecVersion.OpenApi2_0; + + foreach (var path in document.Paths) + { + if (ContainsUnsafeCharacters(path.Key)) + { + diagnostic.Errors.Add(new OpenApiError( + path.Key, + $"Path '{path.Key}' contains illegal characters (quotes, backslashes, or control characters) and is rejected to prevent code injection into Refit attributes. Use --skip-validation to bypass.")); + } + + if (path.Value?.Operations == null) + continue; + + foreach (var operation in path.Value.Operations.Values) + { + if (operation == null) + continue; + + if (operation.Parameters != null) + { + foreach (var parameter in operation.Parameters) + { + if (parameter.In == ParameterLocation.Header && ContainsUnsafeCharacters(parameter.Name)) + { + diagnostic.Errors.Add(new OpenApiError( + parameter.Name ?? string.Empty, + $"Header parameter name '{parameter.Name}' contains illegal characters and is rejected to prevent code injection into Refit attributes. Use --skip-validation to bypass.")); + } + } + } + + if (validateContentTypes) + { + ValidateContentTypeKeys(operation, diagnostic); + } + } + } + } + + private static void ValidateContentTypeKeys(OpenApiOperation operation, OpenApiDiagnostic diagnostic) + { + if (operation.RequestBody?.Content != null) + { + foreach (var contentType in operation.RequestBody.Content.Keys) + { + AddContentTypeErrorIfUnsafe(contentType, diagnostic); + } + } + + if (operation.Responses == null) + return; + + foreach (var response in operation.Responses.Values) + { + if (response?.Content == null) + continue; + + foreach (var contentType in response.Content.Keys) + { + AddContentTypeErrorIfUnsafe(contentType, diagnostic); + } + } + } + + private static void AddContentTypeErrorIfUnsafe(string contentType, OpenApiDiagnostic diagnostic) + { + if (ContainsUnsafeCharacters(contentType)) + { + diagnostic.Errors.Add(new OpenApiError( + contentType, + $"Content type '{contentType}' contains illegal characters and is rejected to prevent code injection into Refit attributes. Use --skip-validation to bypass.")); + } + } +} diff --git a/src/Refitter.Core/Validation/OpenApiValidator.cs b/src/Refitter.Core/Validation/OpenApiValidator.cs index efc016502..39f1f5fab 100644 --- a/src/Refitter.Core/Validation/OpenApiValidator.cs +++ b/src/Refitter.Core/Validation/OpenApiValidator.cs @@ -1,3 +1,5 @@ +using System.Threading; +using System.Threading.Tasks; using Microsoft.OpenApi; using Microsoft.OpenApi.Reader; @@ -26,6 +28,8 @@ public static async Task Validate( var walker = new OpenApiWalker(statsVisitor); walker.Walk(result.OpenApiDocument); + AttributeStringValidator.Validate(result.OpenApiDocument, result.OpenApiDiagnostic); + return new( result.OpenApiDiagnostic, statsVisitor); diff --git a/src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/SwaggerPetstoreMultipleInterfaces.g.cs b/src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/SwaggerPetstoreMultipleInterfaces.g.cs index 4e0d2a833..69ad732c7 100644 --- a/src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/SwaggerPetstoreMultipleInterfaces.g.cs +++ b/src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/SwaggerPetstoreMultipleInterfaces.g.cs @@ -8,7 +8,6 @@ using System.Text.Json.Serialization; using System.Threading; using System.Threading.Tasks; -using System.Net.Http; using Refitter.Tests.AdditionalFiles.SingeInterface; @@ -512,8 +511,9 @@ public partial interface IDeleteUserEndpoint namespace Refitter.Tests.AdditionalFiles.ByEndpoint { using System; - using Microsoft.Extensions.DependencyInjection; - using Refit; +using System.Net.Http; +using Microsoft.Extensions.DependencyInjection; +using Refit; /// /// Extension methods for configuring Refit clients in the service collection. @@ -536,6 +536,7 @@ public static IServiceCollection ConfigureRefitClients( { var clientBuilderIUpdatePetEndpoint = services .AddRefitClient(settings) + .ConfigureHttpClient(c => c.BaseAddress = baseUrl) .AddHttpMessageHandler() .AddHttpMessageHandler(); @@ -544,6 +545,7 @@ public static IServiceCollection ConfigureRefitClients( var clientBuilderIAddPetEndpoint = services .AddRefitClient(settings) + .ConfigureHttpClient(c => c.BaseAddress = baseUrl) .AddHttpMessageHandler() .AddHttpMessageHandler(); @@ -552,6 +554,7 @@ public static IServiceCollection ConfigureRefitClients( var clientBuilderIFindPetsByStatusEndpoint = services .AddRefitClient(settings) + .ConfigureHttpClient(c => c.BaseAddress = baseUrl) .AddHttpMessageHandler() .AddHttpMessageHandler(); @@ -560,6 +563,7 @@ public static IServiceCollection ConfigureRefitClients( var clientBuilderIFindPetsByTagsEndpoint = services .AddRefitClient(settings) + .ConfigureHttpClient(c => c.BaseAddress = baseUrl) .AddHttpMessageHandler() .AddHttpMessageHandler(); @@ -568,6 +572,7 @@ public static IServiceCollection ConfigureRefitClients( var clientBuilderIGetPetByIdEndpoint = services .AddRefitClient(settings) + .ConfigureHttpClient(c => c.BaseAddress = baseUrl) .AddHttpMessageHandler() .AddHttpMessageHandler(); @@ -576,6 +581,7 @@ public static IServiceCollection ConfigureRefitClients( var clientBuilderIUpdatePetWithFormEndpoint = services .AddRefitClient(settings) + .ConfigureHttpClient(c => c.BaseAddress = baseUrl) .AddHttpMessageHandler() .AddHttpMessageHandler(); @@ -584,6 +590,7 @@ public static IServiceCollection ConfigureRefitClients( var clientBuilderIDeletePetEndpoint = services .AddRefitClient(settings) + .ConfigureHttpClient(c => c.BaseAddress = baseUrl) .AddHttpMessageHandler() .AddHttpMessageHandler(); @@ -592,6 +599,7 @@ public static IServiceCollection ConfigureRefitClients( var clientBuilderIUploadFileEndpoint = services .AddRefitClient(settings) + .ConfigureHttpClient(c => c.BaseAddress = baseUrl) .AddHttpMessageHandler() .AddHttpMessageHandler(); @@ -600,6 +608,7 @@ public static IServiceCollection ConfigureRefitClients( var clientBuilderIGetInventoryEndpoint = services .AddRefitClient(settings) + .ConfigureHttpClient(c => c.BaseAddress = baseUrl) .AddHttpMessageHandler() .AddHttpMessageHandler(); @@ -608,6 +617,7 @@ public static IServiceCollection ConfigureRefitClients( var clientBuilderIPlaceOrderEndpoint = services .AddRefitClient(settings) + .ConfigureHttpClient(c => c.BaseAddress = baseUrl) .AddHttpMessageHandler() .AddHttpMessageHandler(); @@ -616,6 +626,7 @@ public static IServiceCollection ConfigureRefitClients( var clientBuilderIGetOrderByIdEndpoint = services .AddRefitClient(settings) + .ConfigureHttpClient(c => c.BaseAddress = baseUrl) .AddHttpMessageHandler() .AddHttpMessageHandler(); @@ -624,6 +635,7 @@ public static IServiceCollection ConfigureRefitClients( var clientBuilderIDeleteOrderEndpoint = services .AddRefitClient(settings) + .ConfigureHttpClient(c => c.BaseAddress = baseUrl) .AddHttpMessageHandler() .AddHttpMessageHandler(); @@ -632,6 +644,7 @@ public static IServiceCollection ConfigureRefitClients( var clientBuilderICreateUserEndpoint = services .AddRefitClient(settings) + .ConfigureHttpClient(c => c.BaseAddress = baseUrl) .AddHttpMessageHandler() .AddHttpMessageHandler(); @@ -640,6 +653,7 @@ public static IServiceCollection ConfigureRefitClients( var clientBuilderICreateUsersWithListInputEndpoint = services .AddRefitClient(settings) + .ConfigureHttpClient(c => c.BaseAddress = baseUrl) .AddHttpMessageHandler() .AddHttpMessageHandler(); @@ -648,6 +662,7 @@ public static IServiceCollection ConfigureRefitClients( var clientBuilderILoginUserEndpoint = services .AddRefitClient(settings) + .ConfigureHttpClient(c => c.BaseAddress = baseUrl) .AddHttpMessageHandler() .AddHttpMessageHandler(); @@ -656,6 +671,7 @@ public static IServiceCollection ConfigureRefitClients( var clientBuilderILogoutUserEndpoint = services .AddRefitClient(settings) + .ConfigureHttpClient(c => c.BaseAddress = baseUrl) .AddHttpMessageHandler() .AddHttpMessageHandler(); @@ -664,6 +680,7 @@ public static IServiceCollection ConfigureRefitClients( var clientBuilderIGetUserByNameEndpoint = services .AddRefitClient(settings) + .ConfigureHttpClient(c => c.BaseAddress = baseUrl) .AddHttpMessageHandler() .AddHttpMessageHandler(); @@ -672,6 +689,7 @@ public static IServiceCollection ConfigureRefitClients( var clientBuilderIUpdateUserEndpoint = services .AddRefitClient(settings) + .ConfigureHttpClient(c => c.BaseAddress = baseUrl) .AddHttpMessageHandler() .AddHttpMessageHandler(); @@ -680,6 +698,7 @@ public static IServiceCollection ConfigureRefitClients( var clientBuilderIDeleteUserEndpoint = services .AddRefitClient(settings) + .ConfigureHttpClient(c => c.BaseAddress = baseUrl) .AddHttpMessageHandler() .AddHttpMessageHandler(); diff --git a/src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/SwaggerPetstoreSingleInterfaceWithHttpResilience.g.cs b/src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/SwaggerPetstoreSingleInterfaceWithHttpResilience.g.cs index 2732b463a..858fc7dbb 100644 --- a/src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/SwaggerPetstoreSingleInterfaceWithHttpResilience.g.cs +++ b/src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/SwaggerPetstoreSingleInterfaceWithHttpResilience.g.cs @@ -7,7 +7,6 @@ using System.Collections.Generic; using System.Text.Json.Serialization; using System.Threading.Tasks; -using System.Net.Http; #nullable enable annotations @@ -743,6 +742,7 @@ public FileParameter(System.IO.Stream data, string fileName, string contentType) namespace Refitter.Tests.AdditionalFiles.SingeInterfaceWithHttpResilience { using System; + using System.Net.Http; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Http.Resilience; using Refit; @@ -766,6 +766,7 @@ public static IServiceCollection ConfigureRefitClients( { var clientBuilderISwaggerPetstoreInterfaceWithHttpResilience = services .AddRefitClient(settings) + .ConfigureHttpClient(c => c.BaseAddress = new Uri("https://petstore3.swagger.io/api/v3")); clientBuilderISwaggerPetstoreInterfaceWithHttpResilience diff --git a/src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/SwaggerPetstoreSingleInterfaceWithPolly.g.cs b/src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/SwaggerPetstoreSingleInterfaceWithPolly.g.cs index e47656499..dc01821c5 100644 --- a/src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/SwaggerPetstoreSingleInterfaceWithPolly.g.cs +++ b/src/Refitter.SourceGenerator.Tests/AdditionalFiles/Generated/SwaggerPetstoreSingleInterfaceWithPolly.g.cs @@ -7,7 +7,6 @@ using System.Collections.Generic; using System.Text.Json.Serialization; using System.Threading.Tasks; -using System.Net.Http; #nullable enable annotations @@ -453,6 +452,7 @@ public FileParameter(System.IO.Stream data, string fileName, string contentType) namespace Refitter.Tests.AdditionalFiles.SingeInterface { using System; + using System.Net.Http; using Microsoft.Extensions.DependencyInjection; using Polly; using Polly.Contrib.WaitAndRetry; diff --git a/src/Refitter.Tests/OpenApi/AttributeStringValidatorTests.cs b/src/Refitter.Tests/OpenApi/AttributeStringValidatorTests.cs new file mode 100644 index 000000000..9c828b252 --- /dev/null +++ b/src/Refitter.Tests/OpenApi/AttributeStringValidatorTests.cs @@ -0,0 +1,189 @@ +using System.Net.Http; +using FluentAssertions; +using Microsoft.OpenApi; +using Microsoft.OpenApi.Reader; +using Refitter.Core.Validation; + +namespace Refitter.Tests.OpenApi; + +public class AttributeStringValidatorTests +{ + [Test] + public void ContainsUnsafeCharacters_Should_Return_False_For_Null_And_Safe_Values() + { + AttributeStringValidator.ContainsUnsafeCharacters(null).Should().BeFalse(); + AttributeStringValidator.ContainsUnsafeCharacters("X-Api-Key").Should().BeFalse(); + } + + [Test] + public void ContainsUnsafeCharacters_Should_Return_True_For_Unsafe_Values() + { + AttributeStringValidator.ContainsUnsafeCharacters("X\"Api").Should().BeTrue(); + AttributeStringValidator.ContainsUnsafeCharacters("X\\Api").Should().BeTrue(); + AttributeStringValidator.ContainsUnsafeCharacters("X\nApi").Should().BeTrue(); + } + + [Test] + public void Validate_Should_Return_When_Document_Is_Null() + { + OpenApiDiagnostic diagnostic = new(); + + AttributeStringValidator.Validate(null, diagnostic); + + diagnostic.Errors.Should().BeEmpty(); + } + + [Test] + public void Validate_Should_Validate_SecurityScheme_And_Return_When_Paths_Are_Null() + { + OpenApiDiagnostic diagnostic = new(); + OpenApiDocument document = new() + { + Components = new OpenApiComponents + { + SecuritySchemes = new Dictionary + { + ["nullScheme"] = null!, + ["unsafeHeader"] = new OpenApiSecurityScheme + { + Type = SecuritySchemeType.ApiKey, + In = ParameterLocation.Header, + Name = "X\"Api" + }, + ["safeHeader"] = new OpenApiSecurityScheme + { + Type = SecuritySchemeType.ApiKey, + In = ParameterLocation.Header, + Name = "X-Api-Key" + }, + ["unsafeQuery"] = new OpenApiSecurityScheme + { + Type = SecuritySchemeType.ApiKey, + In = ParameterLocation.Query, + Name = "X\"Api" + } + } + }, + Paths = null! + }; + + AttributeStringValidator.Validate(document, diagnostic); + + diagnostic.Errors.Should().HaveCount(1); + diagnostic.Errors[0].Pointer.Should().Be("unsafeHeader"); + diagnostic.Errors[0].Message.Should().Contain("Security scheme 'unsafeHeader'"); + } + + [Test] + public void Validate_Should_Report_Path_And_Header_Errors_And_Skip_Null_Branches() + { + OpenApiDiagnostic diagnostic = new(); + OpenApiDocument document = new() + { + Paths = new OpenApiPaths + { + ["/nullpath"] = null!, + ["unsafe\"path"] = new OpenApiPathItem + { + Operations = null! + }, + ["/safe"] = new OpenApiPathItem + { + Operations = new Dictionary + { + [HttpMethod.Get] = new OpenApiOperation + { + Parameters = null! + }, + [HttpMethod.Post] = null!, + [HttpMethod.Put] = new OpenApiOperation + { + Parameters = new List + { + new OpenApiParameter + { + In = ParameterLocation.Header, + Name = "X\"Bad" + }, + new OpenApiParameter + { + In = ParameterLocation.Header, + Name = "X-Good" + }, + new OpenApiParameter + { + In = ParameterLocation.Query, + Name = "X\"Ignored" + } + } + } + } + } + } + }; + + AttributeStringValidator.Validate(document, diagnostic); + + diagnostic.Errors.Should().HaveCount(2); + diagnostic.Errors.Select(x => x.Pointer).Should().Contain(new[] { "unsafe\"path", "X\"Bad" }); + } + + [Test] + public void Validate_Should_Report_ContentType_Errors_For_OpenApi3() + { + OpenApiDiagnostic diagnostic = new() { SpecificationVersion = OpenApiSpecVersion.OpenApi3_0 }; + OpenApiDocument document = BuildContentTypeDocument(); + + AttributeStringValidator.Validate(document, diagnostic); + + diagnostic.Errors.Should().HaveCount(2); + diagnostic.Errors.Select(x => x.Message).Should().OnlyContain(m => m.Contains("Content type")); + diagnostic.Errors.Select(x => x.Pointer).Should().Contain(new[] { "application/json\")] x", "text/plain\"" }); + } + + [Test] + public void Validate_Should_Not_Report_ContentType_Errors_For_OpenApi2() + { + OpenApiDiagnostic diagnostic = new() { SpecificationVersion = OpenApiSpecVersion.OpenApi2_0 }; + OpenApiDocument document = BuildContentTypeDocument(); + + AttributeStringValidator.Validate(document, diagnostic); + + diagnostic.Errors.Should().BeEmpty(); + } + + private static OpenApiDocument BuildContentTypeDocument() => new() + { + Paths = new OpenApiPaths + { + ["/api"] = new OpenApiPathItem + { + Operations = new Dictionary + { + [HttpMethod.Post] = new OpenApiOperation + { + RequestBody = new OpenApiRequestBody + { + Content = new Dictionary + { + ["application/json"] = new OpenApiMediaType(), + ["application/json\")] x"] = new OpenApiMediaType() + } + }, + Responses = new OpenApiResponses + { + ["200"] = new OpenApiResponse + { + Content = new Dictionary + { + ["text/plain\""] = new OpenApiMediaType() + } + }, + ["204"] = null! + } + } + } + } + } + }; +} diff --git a/src/Refitter.Tests/Scenarios/SecurityAttributeInjectionTests.cs b/src/Refitter.Tests/Scenarios/SecurityAttributeInjectionTests.cs new file mode 100644 index 000000000..d6cc60ba3 --- /dev/null +++ b/src/Refitter.Tests/Scenarios/SecurityAttributeInjectionTests.cs @@ -0,0 +1,410 @@ +using FluentAssertions; +using Refitter.Core; +using Refitter.Core.Validation; +using Refitter.Tests.Build; +using Refitter.Tests.TestUtilities; +using TUnit.Core; + +namespace Refitter.Tests.Scenarios; + +// Regression tests for the Refit attribute-injection advisories. An OpenAPI path, header name, or +// content-type key that closes the Refit attribute string and injects a [ModuleInitializer] file +// class must not break out of the generated code, and must be rejected by validation. +// Covers GHSA-3fhm-p725-h3g3 (path), GHSA-58x9-vjvp-6mx8 (header name), GHSA-p32v-8v8j-j534 (content-type). +public class SecurityAttributeInjectionTests +{ + private const string MaliciousPath = + @"/p"")] Task Dummy(); } } file class Evil { [System.Runtime.CompilerServices.ModuleInitializer] internal static void Init(){ } } namespace N2 { public partial interface I2 { [Get(""/q"; + + private const string MaliciousHeaderName = + @"X"")] string a); } file class H {} public partial interface J { [Header(""Y"; + + private const string MaliciousContentType = + @"application/json"")] Task Dummy(); } } file class CtEvil { [System.Runtime.CompilerServices.ModuleInitializer] internal static void Init(){ } } namespace N3 { public partial interface I3 { [Get(""/ctq"; + + private const string PathInjectionSpec = @" +openapi: 3.0.0 +info: + title: Injection + version: 1.0.0 +paths: + '" + MaliciousPath + @"': + get: + operationId: GetU + responses: + '200': + description: ok +"; + + private const string PathInjectionSpecV2 = @" +swagger: '2.0' +info: + title: Injection + version: 1.0.0 +paths: + '" + MaliciousPath + @"': + get: + operationId: GetU + responses: + '200': + description: ok +"; + + private const string HeaderInjectionSpec = @" +openapi: 3.0.0 +info: + title: Injection + version: 1.0.0 +paths: + /api: + get: + operationId: GetU + parameters: + - name: '" + MaliciousHeaderName + @"' + in: header + schema: + type: string + responses: + '200': + description: ok +"; + + private const string HeaderInjectionSpecV2 = @" +swagger: '2.0' +info: + title: Injection + version: 1.0.0 +paths: + /api: + get: + operationId: GetU + parameters: + - name: '" + MaliciousHeaderName + @"' + in: header + type: string + responses: + '200': + description: ok +"; + + private const string SecuritySchemeHeaderInjectionSpec = @" +openapi: 3.0.0 +info: + title: Injection + version: 1.0.0 +paths: + /api: + get: + operationId: GetU + security: + - apiKey: [] + responses: + '200': + description: ok +components: + securitySchemes: + apiKey: + type: apiKey + in: header + name: '" + MaliciousHeaderName + @"' +"; + + private const string SecuritySchemeHeaderInjectionSpecV2 = @" +swagger: '2.0' +info: + title: Injection + version: 1.0.0 +paths: + /api: + get: + operationId: GetU + security: + - apiKey: [] + responses: + '200': + description: ok +securityDefinitions: + apiKey: + type: apiKey + in: header + name: '" + MaliciousHeaderName + @"' +"; + + private const string ContentTypeInjectionSpec = @" +openapi: 3.0.0 +info: + title: Injection + version: 1.0.0 +paths: + /api: + post: + operationId: PostU + requestBody: + content: + '" + MaliciousContentType + @"': + schema: + type: string + responses: + '200': + description: ok + content: + '" + MaliciousContentType + @"': + schema: + type: string +"; + + [Test] + public async Task Path_Injection_Does_Not_Break_Out_Of_Attribute() + { + string generatedCode = await GenerateCode(PathInjectionSpec); + generatedCode.Should().Contain("\\\""); + generatedCode.Should().NotContain("[Get(\"/q\")]"); + } + + [Category("Integration")] + [Test] + public async Task Generated_Code_With_Path_Injection_Is_Inert_And_Compiles() + { + string generatedCode = await GenerateCode(PathInjectionSpec); + BuildHelper.BuildCSharp(generatedCode).Should().BeTrue(); + } + + [Test] + public async Task Header_Injection_Does_Not_Break_Out_Of_Attribute() + { + string generatedCode = await GenerateCode(HeaderInjectionSpec); + generatedCode.Should().Contain("\\\""); + generatedCode.Should().NotContain("[Header(\"Y\")]"); + } + + [Category("Integration")] + [Test] + public async Task Generated_Code_With_Header_Injection_Is_Inert_And_Compiles() + { + string generatedCode = await GenerateCode(HeaderInjectionSpec); + BuildHelper.BuildCSharp(generatedCode).Should().BeTrue(); + } + + [Test] + public async Task Validation_Rejects_Path_With_Breakout_Characters() + { + var swaggerFile = await SwaggerFileHelper.CreateSwaggerFile(PathInjectionSpec); + try + { + var result = await OpenApiValidator.Validate(swaggerFile); + result.IsValid.Should().BeFalse(); + } + finally + { + CleanUp(swaggerFile); + } + } + + [Test] + public async Task Path_Injection_Does_Not_Break_Out_Of_Attribute_V2() + { + string generatedCode = await GenerateCode(PathInjectionSpecV2); + generatedCode.Should().Contain("\\\""); + generatedCode.Should().NotContain("[Get(\"/q\")]"); + } + + [Category("Integration")] + [Test] + public async Task Generated_Code_With_Path_Injection_Is_Inert_And_Compiles_V2() + { + string generatedCode = await GenerateCode(PathInjectionSpecV2); + BuildHelper.BuildCSharp(generatedCode).Should().BeTrue(); + } + + [Test] + public async Task Header_Injection_Does_Not_Break_Out_Of_Attribute_V2() + { + string generatedCode = await GenerateCode(HeaderInjectionSpecV2); + generatedCode.Should().Contain("\\\""); + generatedCode.Should().NotContain("[Header(\"Y\")]"); + } + + [Category("Integration")] + [Test] + public async Task Generated_Code_With_Header_Injection_Is_Inert_And_Compiles_V2() + { + string generatedCode = await GenerateCode(HeaderInjectionSpecV2); + BuildHelper.BuildCSharp(generatedCode).Should().BeTrue(); + } + + [Test] + public async Task Validation_Rejects_Path_With_Breakout_Characters_V2() + { + var swaggerFile = await SwaggerFileHelper.CreateSwaggerFile(PathInjectionSpecV2); + try + { + var result = await OpenApiValidator.Validate(swaggerFile); + result.IsValid.Should().BeFalse(); + } + finally + { + CleanUp(swaggerFile); + } + } + + [Test] + public async Task Validation_Rejects_Header_With_Breakout_Characters() + { + var swaggerFile = await SwaggerFileHelper.CreateSwaggerFile(HeaderInjectionSpec); + try + { + var result = await OpenApiValidator.Validate(swaggerFile); + result.IsValid.Should().BeFalse(); + } + finally + { + CleanUp(swaggerFile); + } + } + + [Test] + public async Task Validation_Rejects_Header_With_Breakout_Characters_V2() + { + var swaggerFile = await SwaggerFileHelper.CreateSwaggerFile(HeaderInjectionSpecV2); + try + { + var result = await OpenApiValidator.Validate(swaggerFile); + result.IsValid.Should().BeFalse(); + } + finally + { + CleanUp(swaggerFile); + } + } + + [Test] + public async Task SecurityScheme_Header_Injection_Does_Not_Break_Out_Of_Attribute() + { + string generatedCode = await GenerateCode( + SecuritySchemeHeaderInjectionSpec, + AuthenticationHeaderStyle.Parameter); + generatedCode.Should().Contain("[Get(\"/api\")]"); + generatedCode.Should().NotContain("[Header(\"Y\")]"); + } + + [Category("Integration")] + [Test] + public async Task Generated_Code_With_SecurityScheme_Header_Injection_Is_Inert_And_Compiles() + { + string generatedCode = await GenerateCode( + SecuritySchemeHeaderInjectionSpec, + AuthenticationHeaderStyle.Parameter); + BuildHelper.BuildCSharp(generatedCode).Should().BeTrue(); + } + + [Test] + public async Task Validation_Rejects_SecurityScheme_Header_With_Breakout_Characters() + { + var swaggerFile = await SwaggerFileHelper.CreateSwaggerFile(SecuritySchemeHeaderInjectionSpec); + try + { + var result = await OpenApiValidator.Validate(swaggerFile); + result.IsValid.Should().BeFalse(); + } + finally + { + CleanUp(swaggerFile); + } + } + + [Test] + public async Task SecurityScheme_Header_Injection_Does_Not_Break_Out_Of_Attribute_V2() + { + string generatedCode = await GenerateCode( + SecuritySchemeHeaderInjectionSpecV2, + AuthenticationHeaderStyle.Parameter); + generatedCode.Should().Contain("[Get(\"/api\")]"); + generatedCode.Should().NotContain("[Header(\"Y\")]"); + } + + [Category("Integration")] + [Test] + public async Task Generated_Code_With_SecurityScheme_Header_Injection_Is_Inert_And_Compiles_V2() + { + string generatedCode = await GenerateCode( + SecuritySchemeHeaderInjectionSpecV2, + AuthenticationHeaderStyle.Parameter); + BuildHelper.BuildCSharp(generatedCode).Should().BeTrue(); + } + + [Test] + public async Task Validation_Rejects_SecurityScheme_Header_With_Breakout_Characters_V2() + { + var swaggerFile = await SwaggerFileHelper.CreateSwaggerFile(SecuritySchemeHeaderInjectionSpecV2); + try + { + var result = await OpenApiValidator.Validate(swaggerFile); + result.IsValid.Should().BeFalse(); + } + finally + { + CleanUp(swaggerFile); + } + } + + [Test] + public async Task ContentType_Injection_Does_Not_Break_Out_Of_Attribute() + { + string generatedCode = await GenerateCode(ContentTypeInjectionSpec); + generatedCode.Should().Contain("\\\""); + generatedCode.Should().NotContain("application/json\")]"); + } + + [Category("Integration")] + [Test] + public async Task Generated_Code_With_ContentType_Injection_Is_Inert_And_Compiles() + { + string generatedCode = await GenerateCode(ContentTypeInjectionSpec); + BuildHelper.BuildCSharp(generatedCode).Should().BeTrue(); + } + + [Test] + public async Task Validation_Rejects_ContentType_With_Breakout_Characters() + { + var swaggerFile = await SwaggerFileHelper.CreateSwaggerFile(ContentTypeInjectionSpec); + try + { + var result = await OpenApiValidator.Validate(swaggerFile); + result.IsValid.Should().BeFalse(); + } + finally + { + CleanUp(swaggerFile); + } + } + + private static async Task GenerateCode( + string spec, + AuthenticationHeaderStyle authenticationHeaderStyle = AuthenticationHeaderStyle.None) + { + var swaggerFile = await SwaggerFileHelper.CreateSwaggerFile(spec); + try + { + var settings = new RefitGeneratorSettings + { + OpenApiPath = swaggerFile, + AuthenticationHeaderStyle = authenticationHeaderStyle + }; + var generator = await RefitGenerator.CreateAsync(settings); + return generator.Generate(); + } + finally + { + CleanUp(swaggerFile); + } + } + + private static void CleanUp(string swaggerFile) + { + if (File.Exists(swaggerFile)) + File.Delete(swaggerFile); + var directory = Path.GetDirectoryName(swaggerFile); + if (directory != null && Directory.Exists(directory)) + Directory.Delete(directory, true); + } +}