From a305d486c7b46dae0143bc21394fb977328faec9 Mon Sep 17 00:00:00 2001 From: Kevin Stubbings Date: Wed, 18 Dec 2024 16:20:10 -0800 Subject: [PATCH 1/4] Add CORS query to C# pack --- csharp/lib/ghsl/ConstExpressions.qll | 67 +++++++++++++++ .../security/CWE-942/CORSMisconfiguration.ql | 54 ++++++++++++ .../CWE-942/CorsMisconfiguration.qhelp | 85 +++++++++++++++++++ .../CorsMisconfigurationCredentials.ql | 42 +++++++++ .../CWE-942/CorsMisconfigurationLib.qll | 46 ++++++++++ .../src/security/CWE-942/examples/CorsBad.cs | 64 ++++++++++++++ .../src/security/CWE-942/examples/CorsGood.cs | 64 ++++++++++++++ .../CWE-942/CorsMiconfigurationCredentials.cs | 25 ++++++ .../security/CWE-942/CorsMisconfiguration.cs | 30 +++++++ .../CWE-942/CorsMisconfiguration.expected | 2 + .../CWE-942/CorsMisconfiguration.qlref | 1 + .../CorsMisconfigurationCredentials.expected | 1 + .../CorsMisconfigurationCredentials.qlref | 1 + csharp/test/security/CWE-942/options | 4 + 14 files changed, 486 insertions(+) create mode 100644 csharp/lib/ghsl/ConstExpressions.qll create mode 100644 csharp/src/security/CWE-942/CORSMisconfiguration.ql create mode 100644 csharp/src/security/CWE-942/CorsMisconfiguration.qhelp create mode 100644 csharp/src/security/CWE-942/CorsMisconfigurationCredentials.ql create mode 100644 csharp/src/security/CWE-942/CorsMisconfigurationLib.qll create mode 100644 csharp/src/security/CWE-942/examples/CorsBad.cs create mode 100644 csharp/src/security/CWE-942/examples/CorsGood.cs create mode 100644 csharp/test/security/CWE-942/CorsMiconfigurationCredentials.cs create mode 100644 csharp/test/security/CWE-942/CorsMisconfiguration.cs create mode 100644 csharp/test/security/CWE-942/CorsMisconfiguration.expected create mode 100644 csharp/test/security/CWE-942/CorsMisconfiguration.qlref create mode 100644 csharp/test/security/CWE-942/CorsMisconfigurationCredentials.expected create mode 100644 csharp/test/security/CWE-942/CorsMisconfigurationCredentials.qlref create mode 100644 csharp/test/security/CWE-942/options diff --git a/csharp/lib/ghsl/ConstExpressions.qll b/csharp/lib/ghsl/ConstExpressions.qll new file mode 100644 index 00000000..323dc410 --- /dev/null +++ b/csharp/lib/ghsl/ConstExpressions.qll @@ -0,0 +1,67 @@ +import csharp + +/** + * Holds if the `Callable` c throws any exception other than `ThrowsArgumentNullException` + */ +predicate callableMayThrowException(Callable c) { + exists(ThrowStmt thre | c = thre.getEnclosingCallable()) and + not callableOnlyThrowsArgumentNullException(c) +} + +/** + * Holds if any exception being thrown by the callable is of type `System.ArgumentNullException` + * It will also hold if no exceptions are thrown by the callable + */ +predicate callableOnlyThrowsArgumentNullException(Callable c) { + forall(ThrowElement thre | c = thre.getEnclosingCallable() | + thre.getThrownExceptionType().hasFullyQualifiedName("System", "ArgumentNullException") + ) +} + +/** + * Hold if the `Expr` e is a `BoolLiteral` with value true, + * the expression has a predictable value == `true`, + * or if it is a `ConditionalExpr` where the `then` and `else` expressions meet `isExpressionAlwaysTrue` criteria + */ +predicate isExpressionAlwaysTrue(Expr e) { + e.(BoolLiteral).getBoolValue() = true + or + e.getValue() = "true" + or + e instanceof ConditionalExpr and + isExpressionAlwaysTrue(e.(ConditionalExpr).getThen()) and + isExpressionAlwaysTrue(e.(ConditionalExpr).getElse()) + or + exists(Callable callable | + callableHasAReturnStmtAndAlwaysReturnsTrue(callable) and + callable.getACall() = e + ) +} + +/** + * Holds if the lambda expression `le` always returns true + */ +predicate lambdaExprReturnsOnlyLiteralTrue(AnonymousFunctionExpr le) { + isExpressionAlwaysTrue(le.getExpressionBody()) +} + +/** + * Holds if the callable has a return statement and it always returns true for all such statements + */ +predicate callableHasAReturnStmtAndAlwaysReturnsTrue(Callable c) { + c.getReturnType() instanceof BoolType and + not callableMayThrowException(c) and + forex(ReturnStmt rs | rs.getEnclosingCallable() = c | + rs.getNumberOfChildren() = 1 and + isExpressionAlwaysTrue(rs.getChildExpr(0)) + ) +} + +/** + * Holds if `c` always returns `true`. + */ +predicate alwaysReturnsTrue(Callable c) { + callableHasAReturnStmtAndAlwaysReturnsTrue(c) + or + lambdaExprReturnsOnlyLiteralTrue(c) +} diff --git a/csharp/src/security/CWE-942/CORSMisconfiguration.ql b/csharp/src/security/CWE-942/CORSMisconfiguration.ql new file mode 100644 index 00000000..6e13f9fc --- /dev/null +++ b/csharp/src/security/CWE-942/CORSMisconfiguration.ql @@ -0,0 +1,54 @@ +/** + * @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 +import CorsMisconfigurationLib + +/** + * 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 the application uses a vulnerable CORS policy. + */ +private predicate hasDangerousOrigins(MethodCall m) { + m.getTarget() + .hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsPolicyBuilder", + "WithOrigins") and + exists(StringLiteral idStr | + idStr.getValue().toLowerCase().matches(["null", "*"]) and + TaintTracking::localExprTaint(idStr, m.getAnArgument()) + ) +} + +from MethodCall add_policy, MethodCall child +where + ( + usedPolicy(add_policy) and + // Misconfigured origin affects used policy + getCallableFromExpr(add_policy.getArgument(1)).calls*(child.getTarget()) + or + add_policy + .getTarget() + .hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsOptions", + "AddDefaultPolicy") and + // Misconfigured origin affects added default policy + getCallableFromExpr(add_policy.getArgument(0)).calls*(child.getTarget()) + ) and + (hasDangerousOrigins(child) or allowAnyOrigin(child)) +select add_policy, "The following CORS policy may allow requests from 3rd party websites" diff --git a/csharp/src/security/CWE-942/CorsMisconfiguration.qhelp b/csharp/src/security/CWE-942/CorsMisconfiguration.qhelp new file mode 100644 index 00000000..3ab8d7fb --- /dev/null +++ b/csharp/src/security/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/src/security/CWE-942/CorsMisconfigurationCredentials.ql b/csharp/src/security/CWE-942/CorsMisconfigurationCredentials.ql new file mode 100644 index 00000000..0252830a --- /dev/null +++ b/csharp/src/security/CWE-942/CorsMisconfigurationCredentials.ql @@ -0,0 +1,42 @@ +/** + * @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 +import CorsMisconfigurationLib + +/** A call to `CorsPolicyBuilder.AllowCredentials`. */ +class AllowsCredentials extends MethodCall { + AllowsCredentials() { + this.getTarget() + .hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsPolicyBuilder", + "AllowCredentials") + } +} + +from MethodCall add_policy, MethodCall setIsOriginAllowed, AllowsCredentials allowsCredentials +where + ( + getCallableFromExpr(add_policy.getArgument(1)).calls*(setIsOriginAllowed.getTarget()) and + usedPolicy(add_policy) and + getCallableFromExpr(add_policy.getArgument(1)).calls*(allowsCredentials.getTarget()) + or + add_policy + .getTarget() + .hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsOptions", + "AddDefaultPolicy") and + getCallableFromExpr(add_policy.getArgument(0)).calls*(setIsOriginAllowed.getTarget()) and + getCallableFromExpr(add_policy.getArgument(0)).calls*(allowsCredentials.getTarget()) + ) and + setIsOriginAllowedReturnsTrue(setIsOriginAllowed) +select add_policy, + "The following CORS policy may allow credentialed requests from 3rd party websites" diff --git a/csharp/src/security/CWE-942/CorsMisconfigurationLib.qll b/csharp/src/security/CWE-942/CorsMisconfigurationLib.qll new file mode 100644 index 00000000..baf189f9 --- /dev/null +++ b/csharp/src/security/CWE-942/CorsMisconfigurationLib.qll @@ -0,0 +1,46 @@ +import csharp +import DataFlow +import ghsl.ConstExpressions + +/** + * Gets the actual callable corresponding to the expression `e`. + */ +Callable getCallableFromExpr(Expr e) { + exists(Expr dcArg | dcArg = e.(DelegateCreation).getArgument() | + result = dcArg.(CallableAccess).getTarget() or + result = dcArg.(AnonymousFunctionExpr) + ) + or + result = e +} + +/** + * Holds if SetIsOriginAllowed always returns true. This sets the Access-Control-Allow-Origin to the requester + */ +predicate setIsOriginAllowedReturnsTrue(MethodCall mc) { + mc.getTarget() + .hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsPolicyBuilder", + "SetIsOriginAllowed") and + alwaysReturnsTrue(mc.getArgument(0)) +} + +/** + * Holds if UseCors is called with the relevant cors policy + */ +predicate usedPolicy(MethodCall add_policy) { + exists(MethodCall uc | + uc.getTarget() + .hasFullyQualifiedName("Microsoft.AspNetCore.Builder.CorsMiddlewareExtensions", "UseCors") and + ( + // Same hardcoded name + uc.getArgument(1).getValue() = add_policy.getArgument(0).getValue() or + // Same variable access + uc.getArgument(1).(VariableAccess).getTarget() = + add_policy.getArgument(0).(VariableAccess).getTarget() or + DataFlow::localExprFlow(add_policy.getArgument(0), uc.getArgument(1)) + ) + ) and + add_policy + .getTarget() + .hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsOptions", "AddPolicy") +} diff --git a/csharp/src/security/CWE-942/examples/CorsBad.cs b/csharp/src/security/CWE-942/examples/CorsBad.cs new file mode 100644 index 00000000..214dbcd5 --- /dev/null +++ b/csharp/src/security/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/src/security/CWE-942/examples/CorsGood.cs b/csharp/src/security/CWE-942/examples/CorsGood.cs new file mode 100644 index 00000000..5bd5013d --- /dev/null +++ b/csharp/src/security/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/test/security/CWE-942/CorsMiconfigurationCredentials.cs b/csharp/test/security/CWE-942/CorsMiconfigurationCredentials.cs new file mode 100644 index 00000000..8565421e --- /dev/null +++ b/csharp/test/security/CWE-942/CorsMiconfigurationCredentials.cs @@ -0,0 +1,25 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Mvc; +using System; +using Microsoft.Extensions.DependencyInjection; + +public class Startup { + public void ConfigureServices(string[] args) { + 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/test/security/CWE-942/CorsMisconfiguration.cs b/csharp/test/security/CWE-942/CorsMisconfiguration.cs new file mode 100644 index 00000000..bc1456b5 --- /dev/null +++ b/csharp/test/security/CWE-942/CorsMisconfiguration.cs @@ -0,0 +1,30 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Mvc; +using System; +using Microsoft.Extensions.DependencyInjection; + +public class Test { + public void ConfigureServices(string[] args) { + var builder = WebApplication.CreateBuilder(args); + var MyAllowSpecificOrigins = "_myAllowSpecificOrigins"; + + builder.Services.AddCors(options => { + options.AddPolicy(MyAllowSpecificOrigins, + policy => { + policy.AllowAnyOrigin().AllowAnyHeader().AllowAnyMethod(); + }); + options.AddDefaultPolicy( + builder => builder + .WithOrigins(["*"]) + .AllowAnyMethod() + .AllowAnyHeader()); + }); + + var app = builder.Build(); + + app.MapGet("/", () => "Hello World!"); + app.UseCors(MyAllowSpecificOrigins); + + app.Run(); + } +} \ No newline at end of file diff --git a/csharp/test/security/CWE-942/CorsMisconfiguration.expected b/csharp/test/security/CWE-942/CorsMisconfiguration.expected new file mode 100644 index 00000000..b874b100 --- /dev/null +++ b/csharp/test/security/CWE-942/CorsMisconfiguration.expected @@ -0,0 +1,2 @@ +| CorsMisconfiguration.cs:12:7:15:10 | call to method AddPolicy | The following CORS policy may allow requests from 3rd party websites | +| CorsMisconfiguration.cs:16:7:20:26 | call to method AddDefaultPolicy | The following CORS policy may allow requests from 3rd party websites | diff --git a/csharp/test/security/CWE-942/CorsMisconfiguration.qlref b/csharp/test/security/CWE-942/CorsMisconfiguration.qlref new file mode 100644 index 00000000..cadcb050 --- /dev/null +++ b/csharp/test/security/CWE-942/CorsMisconfiguration.qlref @@ -0,0 +1 @@ +experimental/CWE-942/CorsMisconfiguration.ql \ No newline at end of file diff --git a/csharp/test/security/CWE-942/CorsMisconfigurationCredentials.expected b/csharp/test/security/CWE-942/CorsMisconfigurationCredentials.expected new file mode 100644 index 00000000..161cf001 --- /dev/null +++ b/csharp/test/security/CWE-942/CorsMisconfigurationCredentials.expected @@ -0,0 +1 @@ +| CorsMiconfigurationCredentials.cs:12:7:15:10 | call to method AddPolicy | The following CORS policy may allow credentialed requests from 3rd party websites | diff --git a/csharp/test/security/CWE-942/CorsMisconfigurationCredentials.qlref b/csharp/test/security/CWE-942/CorsMisconfigurationCredentials.qlref new file mode 100644 index 00000000..5b17285c --- /dev/null +++ b/csharp/test/security/CWE-942/CorsMisconfigurationCredentials.qlref @@ -0,0 +1 @@ +experimental/CWE-942/CorsMisconfigurationCredentials.ql \ No newline at end of file diff --git a/csharp/test/security/CWE-942/options b/csharp/test/security/CWE-942/options new file mode 100644 index 00000000..f6d138f8 --- /dev/null +++ b/csharp/test/security/CWE-942/options @@ -0,0 +1,4 @@ +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 + From c4bd7ca1dc9dcd3b3e263ae9cbd3a4af6994c94e Mon Sep 17 00:00:00 2001 From: Kevin Stubbings Date: Thu, 19 Dec 2024 10:21:21 -0800 Subject: [PATCH 2/4] Fixed pack issues --- csharp/lib/ghsl/ConstExpressions.qll | 67 ------------------- .../CWE-942/CorsMisconfigurationLib.qll | 4 +- .../CWE-942/CorsMisconfiguration.qlref | 2 +- .../CorsMisconfigurationCredentials.qlref | 2 +- 4 files changed, 4 insertions(+), 71 deletions(-) delete mode 100644 csharp/lib/ghsl/ConstExpressions.qll diff --git a/csharp/lib/ghsl/ConstExpressions.qll b/csharp/lib/ghsl/ConstExpressions.qll deleted file mode 100644 index 323dc410..00000000 --- a/csharp/lib/ghsl/ConstExpressions.qll +++ /dev/null @@ -1,67 +0,0 @@ -import csharp - -/** - * Holds if the `Callable` c throws any exception other than `ThrowsArgumentNullException` - */ -predicate callableMayThrowException(Callable c) { - exists(ThrowStmt thre | c = thre.getEnclosingCallable()) and - not callableOnlyThrowsArgumentNullException(c) -} - -/** - * Holds if any exception being thrown by the callable is of type `System.ArgumentNullException` - * It will also hold if no exceptions are thrown by the callable - */ -predicate callableOnlyThrowsArgumentNullException(Callable c) { - forall(ThrowElement thre | c = thre.getEnclosingCallable() | - thre.getThrownExceptionType().hasFullyQualifiedName("System", "ArgumentNullException") - ) -} - -/** - * Hold if the `Expr` e is a `BoolLiteral` with value true, - * the expression has a predictable value == `true`, - * or if it is a `ConditionalExpr` where the `then` and `else` expressions meet `isExpressionAlwaysTrue` criteria - */ -predicate isExpressionAlwaysTrue(Expr e) { - e.(BoolLiteral).getBoolValue() = true - or - e.getValue() = "true" - or - e instanceof ConditionalExpr and - isExpressionAlwaysTrue(e.(ConditionalExpr).getThen()) and - isExpressionAlwaysTrue(e.(ConditionalExpr).getElse()) - or - exists(Callable callable | - callableHasAReturnStmtAndAlwaysReturnsTrue(callable) and - callable.getACall() = e - ) -} - -/** - * Holds if the lambda expression `le` always returns true - */ -predicate lambdaExprReturnsOnlyLiteralTrue(AnonymousFunctionExpr le) { - isExpressionAlwaysTrue(le.getExpressionBody()) -} - -/** - * Holds if the callable has a return statement and it always returns true for all such statements - */ -predicate callableHasAReturnStmtAndAlwaysReturnsTrue(Callable c) { - c.getReturnType() instanceof BoolType and - not callableMayThrowException(c) and - forex(ReturnStmt rs | rs.getEnclosingCallable() = c | - rs.getNumberOfChildren() = 1 and - isExpressionAlwaysTrue(rs.getChildExpr(0)) - ) -} - -/** - * Holds if `c` always returns `true`. - */ -predicate alwaysReturnsTrue(Callable c) { - callableHasAReturnStmtAndAlwaysReturnsTrue(c) - or - lambdaExprReturnsOnlyLiteralTrue(c) -} diff --git a/csharp/src/security/CWE-942/CorsMisconfigurationLib.qll b/csharp/src/security/CWE-942/CorsMisconfigurationLib.qll index baf189f9..63037b65 100644 --- a/csharp/src/security/CWE-942/CorsMisconfigurationLib.qll +++ b/csharp/src/security/CWE-942/CorsMisconfigurationLib.qll @@ -1,6 +1,6 @@ import csharp import DataFlow -import ghsl.ConstExpressions +import security.JsonWebTokenHandler.JsonWebTokenHandlerLib /** * Gets the actual callable corresponding to the expression `e`. @@ -21,7 +21,7 @@ predicate setIsOriginAllowedReturnsTrue(MethodCall mc) { mc.getTarget() .hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsPolicyBuilder", "SetIsOriginAllowed") and - alwaysReturnsTrue(mc.getArgument(0)) + mc.getArgument(0) instanceof CallableAlwaysReturnsTrue } /** diff --git a/csharp/test/security/CWE-942/CorsMisconfiguration.qlref b/csharp/test/security/CWE-942/CorsMisconfiguration.qlref index cadcb050..21334b01 100644 --- a/csharp/test/security/CWE-942/CorsMisconfiguration.qlref +++ b/csharp/test/security/CWE-942/CorsMisconfiguration.qlref @@ -1 +1 @@ -experimental/CWE-942/CorsMisconfiguration.ql \ No newline at end of file +security/CWE-942/CorsMisconfiguration.ql \ No newline at end of file diff --git a/csharp/test/security/CWE-942/CorsMisconfigurationCredentials.qlref b/csharp/test/security/CWE-942/CorsMisconfigurationCredentials.qlref index 5b17285c..aaab9848 100644 --- a/csharp/test/security/CWE-942/CorsMisconfigurationCredentials.qlref +++ b/csharp/test/security/CWE-942/CorsMisconfigurationCredentials.qlref @@ -1 +1 @@ -experimental/CWE-942/CorsMisconfigurationCredentials.ql \ No newline at end of file +security/CWE-942/CorsMisconfigurationCredentials.ql \ No newline at end of file From 13d45d380d46ea4b7081822b2d39bee849d30a5b Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 20 Dec 2024 08:27:39 +0100 Subject: [PATCH 3/4] C#: Adjust options to point to the stub location within the checked out codeql repo. --- csharp/test/security/CWE-942/options | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/csharp/test/security/CWE-942/options b/csharp/test/security/CWE-942/options index f6d138f8..20f9a571 100644 --- a/csharp/test/security/CWE-942/options +++ b/csharp/test/security/CWE-942/options @@ -1,4 +1,4 @@ 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:${testdir}/../../../../codeql/csharp/ql/test/resources/stubs/_frameworks/Microsoft.NETCore.App/Microsoft.NETCore.App.csproj +semmle-extractor-options: --load-sources-from-project:${testdir}/../../../../codeql/csharp/ql/test/resources/stubs/_frameworks/Microsoft.AspNetCore.App/Microsoft.AspNetCore.App.csproj From ae4b59f7d03ff50b7eab1b01076e208b6e7c5941 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 20 Dec 2024 08:56:14 +0100 Subject: [PATCH 4/4] C#: Rename query file to align with other files (and to fix casing issue in .qlref file). --- .../CWE-942/{CORSMisconfiguration.ql => CorsMisconfiguration.ql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename csharp/src/security/CWE-942/{CORSMisconfiguration.ql => CorsMisconfiguration.ql} (100%) diff --git a/csharp/src/security/CWE-942/CORSMisconfiguration.ql b/csharp/src/security/CWE-942/CorsMisconfiguration.ql similarity index 100% rename from csharp/src/security/CWE-942/CORSMisconfiguration.ql rename to csharp/src/security/CWE-942/CorsMisconfiguration.ql