diff --git a/csharp/ql/src/change-notes/2024-11-26-cors-query.md b/csharp/ql/src/change-notes/2024-11-26-cors-query.md new file mode 100644 index 000000000000..77df5ba3a398 --- /dev/null +++ b/csharp/ql/src/change-notes/2024-11-26-cors-query.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* C#: Added `ccs/web/cors-misconfiguration` queries which looks for CORS misconfigurations with and without credentials. \ No newline at end of file diff --git a/csharp/ql/src/experimental/CWE-942/CorsMisconfiguration.qhelp b/csharp/ql/src/experimental/CWE-942/CorsMisconfiguration.qhelp new file mode 100644 index 000000000000..3ab8d7fbba93 --- /dev/null +++ b/csharp/ql/src/experimental/CWE-942/CorsMisconfiguration.qhelp @@ -0,0 +1,85 @@ + + + + +

+ + A server can send the + "Access-Control-Allow-Credentials" CORS header to control + when a browser may send user credentials in Cross-Origin HTTP + requests. + +

+

+ + When the Access-Control-Allow-Credentials header + is "true", the Access-Control-Allow-Origin + header must have a value different from "*" in order to + make browsers accept the header. Therefore, to allow multiple origins + for Cross-Origin requests with credentials, the server must + dynamically compute the value of the + "Access-Control-Allow-Origin" header. Computing this + header value from information in the request to the server can + therefore potentially allow an attacker to control the origins that + the browser sends credentials to. + +

+ + + +
+ + +

+ + When the Access-Control-Allow-Credentials header + value is "true", a dynamic computation of the + Access-Control-Allow-Origin header must involve + sanitization if it relies on user-controlled input. + + +

+

+ + Since the "null" origin is easy to obtain for an + attacker, it is never safe to use "null" as the value of + the Access-Control-Allow-Origin header when the + Access-Control-Allow-Credentials header value is + "true". + +

+
+ + +

+ + In the example below, the server allows the browser to send + user credentials in a Cross-Origin request. The request header + origins controls the allowed origins for such a + Cross-Origin request. + +

+ + + +

+ + This is not secure, since an attacker can choose the value of + the origin request header to make the browser send + credentials to their own server. The use of a allowlist containing + allowed origins for the Cross-Origin request fixes the issue: + +

+ + +
+ + +
  • Mozilla Developer Network: CORS, Access-Control-Allow-Origin.
  • +
  • Mozilla Developer Network: CORS, Access-Control-Allow-Credentials.
  • +
  • PortSwigger: Exploiting CORS Misconfigurations for Bitcoins and Bounties
  • +
  • W3C: CORS for developers, Advice for Resource Owners
  • +
    +
    diff --git a/csharp/ql/src/experimental/CWE-942/CorsMisconfiguration.ql b/csharp/ql/src/experimental/CWE-942/CorsMisconfiguration.ql new file mode 100644 index 000000000000..8f75a4e7787a --- /dev/null +++ b/csharp/ql/src/experimental/CWE-942/CorsMisconfiguration.ql @@ -0,0 +1,88 @@ +/** + * @name CORS misconfiguration + * @description Keeping an open CORS policy may result in security issues as third party website may be able to + * access other websites. + * @kind problem + * @problem.severity error + * @security-severity 7.5 + * @precision high + * @id cs/web/cors-misconfiguration + * @tags security + * external/cwe/cwe-942 + */ + +import csharp +private import DataFlow +import semmle.code.csharp.frameworks.system.Web + +/** + * Holds if SetIsOriginAllowed always returns true. This sets the Access-Control-Allow-Origin to the requester + */ +private predicate functionAlwaysReturnsTrue(MethodCall mc) { + mc.getTarget() + .hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsPolicyBuilder", + "SetIsOriginAllowed") and + alwaysReturnsTrue(mc.getArgument(0)) +} + +/** + * Holds if `c` always returns `true`. + */ +private predicate alwaysReturnsTrue(Callable c) { + forex(ReturnStmt rs | rs.getEnclosingCallable() = c | + rs.getExpr().(BoolLiteral).getBoolValue() = true + ) + or + c.getBody().(BoolLiteral).getBoolValue() = true +} + +/** + * Holds if the application uses a vulnerable CORS policy. + */ +private predicate hasDangerousOrigins(MethodCall m) { + m.getTarget() + .hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsPolicyBuilder", + "WithOrigins") and + m.getAnArgument().getValue() = ["null", "*"] +} + +/** + * Holds if the application allows an origin using "*" origin. + */ +private predicate allowAnyOrigin(MethodCall m) { + m.getTarget() + .hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsPolicyBuilder", + "AllowAnyOrigin") +} + +/** + * Holds if UseCors is called with the revlevant cors policy + */ +private predicate configIsUsed(MethodCall add_policy) { + exists(MethodCall uc | + uc.getTarget() + .hasFullyQualifiedName("Microsoft.AspNetCore.Builder.CorsMiddlewareExtensions", "UseCors") and + ( + uc.getArgument(1).getValue() = add_policy.getArgument(0).getValue() or + localFlow(DataFlow::exprNode(add_policy.getArgument(0)), DataFlow::exprNode(uc.getArgument(1))) + ) + ) +} + +from MethodCall add_policy, MethodCall m +where + ( + add_policy + .getTarget() + .hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsOptions", "AddPolicy") and + add_policy.getArgument(1) = m.getParent*() and + configIsUsed(add_policy) + or + add_policy + .getTarget() + .hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsOptions", + "AddDefaultPolicy") and + add_policy.getArgument(0) = m.getParent*() + ) and + (hasDangerousOrigins(m) or allowAnyOrigin(m) or functionAlwaysReturnsTrue(m)) +select add_policy, "The following CORS policy may be vulnerable to 3rd party websites" diff --git a/csharp/ql/src/experimental/CWE-942/CorsMisconfigurationCredentials.ql b/csharp/ql/src/experimental/CWE-942/CorsMisconfigurationCredentials.ql new file mode 100644 index 000000000000..e1288fd0c890 --- /dev/null +++ b/csharp/ql/src/experimental/CWE-942/CorsMisconfigurationCredentials.ql @@ -0,0 +1,84 @@ +/** + * @name Credentialed CORS Misconfiguration + * @description Allowing any origin while allowing credentials may result in security issues as third party website may be able to + * access private resources. + * @kind problem + * @problem.severity error + * @security-severity 7.5 + * @precision high + * @id cs/web/cors-misconfiguration-credentials + * @tags security + * external/cwe/cwe-942 + */ + +import csharp +private import DataFlow +import semmle.code.csharp.frameworks.system.Web + +/** + * Holds if credentials are allowed + */ +private predicate allowsCredentials(MethodCall credentials) { + credentials + .getTarget() + .hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsPolicyBuilder", + "AllowCredentials") +} + +/** + * Holds if SetIsOriginAllowed always returns true. This sets the Access-Control-Allow-Origin to the requester + */ +private predicate functionAlwaysReturnsTrue(MethodCall mc) { + mc.getTarget() + .hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsPolicyBuilder", + "SetIsOriginAllowed") and + alwaysReturnsTrue(mc.getArgument(0)) +} + +/** + * Holds if `c` always returns `true`. + */ +private predicate alwaysReturnsTrue(Callable c) { + forex(ReturnStmt rs | rs.getEnclosingCallable() = c | + rs.getExpr().(BoolLiteral).getBoolValue() = true + ) + or + c.getBody().(BoolLiteral).getBoolValue() = true +} + +/** + * Holds if UseCors is called with the revlevant cors policy + */ +private predicate configIsUsed(MethodCall add_policy) { + exists(MethodCall uc | + uc.getTarget() + .hasFullyQualifiedName("Microsoft.AspNetCore.Builder.CorsMiddlewareExtensions", "UseCors") and + ( + uc.getArgument(1).getValue() = add_policy.getArgument(0).getValue() or + uc.getArgument(1).(VariableAccess).getTarget() = + add_policy.getArgument(0).(VariableAccess).getTarget() or + localFlow(DataFlow::exprNode(add_policy.getArgument(0)), DataFlow::exprNode(uc.getArgument(1))) + ) + ) +} + +from MethodCall add_policy, MethodCall m, MethodCall allowsCredentials +where + ( + add_policy + .getTarget() + .hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsOptions", "AddPolicy") and + add_policy.getArgument(1) = m.getParent*() and + configIsUsed(add_policy) and + add_policy.getArgument(1) = allowsCredentials.getParent*() + or + add_policy + .getTarget() + .hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsOptions", + "AddDefaultPolicy") and + add_policy.getArgument(0) = m.getParent*() and + add_policy.getArgument(0) = allowsCredentials.getParent*() + ) and + (allowsCredentials(allowsCredentials) and functionAlwaysReturnsTrue(m)) +select add_policy, + "The following CORS policy may allow credentialed requests from 3rd party websites" diff --git a/csharp/ql/src/experimental/CWE-942/examples/CorsBad.cs b/csharp/ql/src/experimental/CWE-942/examples/CorsBad.cs new file mode 100644 index 000000000000..214dbcd5263e --- /dev/null +++ b/csharp/ql/src/experimental/CWE-942/examples/CorsBad.cs @@ -0,0 +1,64 @@ +using Leaf.Middlewares; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; + +namespace Leaf +{ + public class Startup + { + public Startup(IConfiguration configuration) + { + Configuration = configuration; + } + + public IConfiguration Configuration { get; } + + // This method gets called by the runtime. Use this method to add services to the container. + public void ConfigureServices(IServiceCollection services) + { + services.AddControllers(); + //services.AddTransient(_ => new MySqlConnection(Configuration["ConnectionStrings:Default"])); + services.AddControllersWithViews() + .AddNewtonsoftJson(options => + options.SerializerSettings.ReferenceLoopHandling = Newtonsoft.Json.ReferenceLoopHandling.Ignore + ); + + services.AddCors(options => { + options.AddPolicy("AllowPolicy", builder => builder + .WithOrigins("null") + .AllowCredentials() + .AllowAnyMethod() + .AllowAnyHeader()); + }); + + } + + // This method gets called by the runtime. Use this method to configure the HTTP request pipeline. + public void Configure(IApplicationBuilder app, IWebHostEnvironment env) + { + app.UseRouting(); + + app.UseCors("AllowPolicy"); + + app.UseRequestResponseLogging(); + + if (env.IsDevelopment()) + { + app.UseDeveloperExceptionPage(); + } + + app.UseHttpsRedirection(); + + app.UseAuthorization(); + + app.UseEndpoints(endpoints => + { + endpoints.MapControllers(); + }); + + } + } +} diff --git a/csharp/ql/src/experimental/CWE-942/examples/CorsGood.cs b/csharp/ql/src/experimental/CWE-942/examples/CorsGood.cs new file mode 100644 index 000000000000..5bd5013d0e68 --- /dev/null +++ b/csharp/ql/src/experimental/CWE-942/examples/CorsGood.cs @@ -0,0 +1,64 @@ +using Leaf.Middlewares; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; + +namespace Leaf +{ + public class Startup + { + public Startup(IConfiguration configuration) + { + Configuration = configuration; + } + + public IConfiguration Configuration { get; } + + // This method gets called by the runtime. Use this method to add services to the container. + public void ConfigureServices(IServiceCollection services) + { + services.AddControllers(); + //services.AddTransient(_ => new MySqlConnection(Configuration["ConnectionStrings:Default"])); + services.AddControllersWithViews() + .AddNewtonsoftJson(options => + options.SerializerSettings.ReferenceLoopHandling = Newtonsoft.Json.ReferenceLoopHandling.Ignore + ); + + services.AddCors(options => { + options.AddPolicy("AllowPolicy", builder => builder + .WithOrigins("http://example.com") + .AllowCredentials() + .AllowAnyMethod() + .AllowAnyHeader()); + }); + + } + + // This method gets called by the runtime. Use this method to configure the HTTP request pipeline. + public void Configure(IApplicationBuilder app, IWebHostEnvironment env) + { + app.UseRouting(); + + app.UseCors("AllowPolicy"); + + app.UseRequestResponseLogging(); + + if (env.IsDevelopment()) + { + app.UseDeveloperExceptionPage(); + } + + app.UseHttpsRedirection(); + + app.UseAuthorization(); + + app.UseEndpoints(endpoints => + { + endpoints.MapControllers(); + }); + + } + } +} diff --git a/csharp/ql/test/experimental/CWE-942/CorsMiconfigurationCredentials.cs b/csharp/ql/test/experimental/CWE-942/CorsMiconfigurationCredentials.cs new file mode 100644 index 000000000000..c5712603d7fd --- /dev/null +++ b/csharp/ql/test/experimental/CWE-942/CorsMiconfigurationCredentials.cs @@ -0,0 +1,25 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Mvc; +using System; + +var builder = WebApplication.CreateBuilder(args); +var MyAllowSpecificOrigins = "_myAllowSpecificOrigins"; + + +builder.Services.AddCors(options => +{ + options.AddPolicy(MyAllowSpecificOrigins, + policy => + { + policy.SetIsOriginAllowed(test => true).AllowCredentials().AllowAnyHeader().AllowAnyMethod(); + }); +}); + +var app = builder.Build(); + + + +app.MapGet("/", () => "Hello World!"); +app.UseCors(MyAllowSpecificOrigins); + +app.Run(); \ No newline at end of file diff --git a/csharp/ql/test/experimental/CWE-942/CorsMisconfiguration.expected b/csharp/ql/test/experimental/CWE-942/CorsMisconfiguration.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/csharp/ql/test/experimental/CWE-942/CorsMisconfigurationCredentials.qlref b/csharp/ql/test/experimental/CWE-942/CorsMisconfigurationCredentials.qlref new file mode 100644 index 000000000000..5b17285c64b0 --- /dev/null +++ b/csharp/ql/test/experimental/CWE-942/CorsMisconfigurationCredentials.qlref @@ -0,0 +1 @@ +experimental/CWE-942/CorsMisconfigurationCredentials.ql \ No newline at end of file diff --git a/csharp/ql/test/experimental/CWE-942/options b/csharp/ql/test/experimental/CWE-942/options new file mode 100644 index 000000000000..67d8fa711a4a --- /dev/null +++ b/csharp/ql/test/experimental/CWE-942/options @@ -0,0 +1,5 @@ +semmle-extractor-options: /nostdlib /noconfig +semmle-extractor-options: --load-sources-from-project:${testdir}/../../resources/stubs/_frameworks/Microsoft.NETCore.App/Microsoft.NETCore.App.csproj +semmle-extractor-options: --load-sources-from-project:../../resources/stubs/_frameworks/Microsoft.AspNetCore.App/Microsoft.AspNetCore.App.csproj +semmle-extractor-options: --load-sources-from-project:../../resources/stubs/Microsoft.Extensions.DependencyInjection.Abstractions/6.0.0/Microsoft.Extensions.DependencyInjection.Abstractions.csproj +