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..1dc4e4cb98d1 --- /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 + * @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(); + }); + + } + } +}