Skip to content

Commit

Permalink
Second round feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
Kwstubbs committed Dec 17, 2024
1 parent 2a4ed45 commit 374eec5
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 15 deletions.
14 changes: 5 additions & 9 deletions csharp/ql/src/experimental/CWE-942/CorsMisconfiguration.ql
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,9 @@ private predicate hasDangerousOrigins(MethodCall m) {
m.getTarget()
.hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsPolicyBuilder",
"WithOrigins") and
(
m.getAnArgument().getValue() = ["null", "*"]
or
exists(StringLiteral idStr |
idStr.getValue().toLowerCase().matches(["null", "*"]) and
DataFlow::localExprFlow(idStr, m.getAnArgument())
)
exists(StringLiteral idStr |
idStr.getValue().toLowerCase().matches(["null", "*"]) and
TaintTracking::localExprTaint(idStr, m.getAnArgument())
)
}

Expand All @@ -45,14 +41,14 @@ where
(
usedPolicy(add_policy) and
// Misconfigured origin affects used policy
add_policy.getArgument(1) = child.getParent*()
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
add_policy.getArgument(0) = child.getParent*()
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"
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,16 @@ class AllowsCredentials extends MethodCall {
from MethodCall add_policy, MethodCall setIsOriginAllowed, AllowsCredentials allowsCredentials
where
(
add_policy.getArgument(1) = setIsOriginAllowed.getParent*() and
getCallableFromExpr(add_policy.getArgument(1)).calls*(setIsOriginAllowed.getTarget()) and
usedPolicy(add_policy) and
add_policy.getArgument(1) = allowsCredentials.getParent*()
getCallableFromExpr(add_policy.getArgument(1)).calls*(allowsCredentials.getTarget())
or
add_policy
.getTarget()
.hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsOptions",
"AddDefaultPolicy") and
add_policy.getArgument(0) = setIsOriginAllowed.getParent*() and
add_policy.getArgument(0) = allowsCredentials.getParent*()
getCallableFromExpr(add_policy.getArgument(0)).calls*(setIsOriginAllowed.getTarget()) and
getCallableFromExpr(add_policy.getArgument(0)).calls*(allowsCredentials.getTarget())
) and
setIsOriginAllowedReturnsTrue(setIsOriginAllowed)
select add_policy,
Expand Down
12 changes: 12 additions & 0 deletions csharp/ql/src/experimental/CWE-942/CorsMisconfigurationLib.qll
Original file line number Diff line number Diff line change
@@ -1,6 +1,18 @@
import csharp
import DataFlow

/**
* 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 the `Callable` c throws any exception other than `ThrowsArgumentNullException`
*/
Expand Down
7 changes: 6 additions & 1 deletion csharp/ql/test/experimental/CWE-942/CorsMisconfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,13 @@ public void ConfigureServices(string[] args) {
builder.Services.AddCors(options => {
options.AddPolicy(MyAllowSpecificOrigins,
policy => {
policy.AllowAnyOrigin().AllowCredentials().AllowAnyHeader().AllowAnyMethod();
policy.AllowAnyOrigin().AllowAnyHeader().AllowAnyMethod();
});
options.AddDefaultPolicy(
builder => builder
.WithOrigins(["*"])
.AllowAnyMethod()
.AllowAnyHeader());
});

var app = builder.Build();
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +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 |
Original file line number Diff line number Diff line change
@@ -1 +1 @@
| CorsMiconfigurationCredentials.cs:18:5:22:24 | call to method AddPolicy | The following CORS policy may allow credentialed requests from 3rd party websites |
| CorsMiconfigurationCredentials.cs:12:7:15:10 | call to method AddPolicy | The following CORS policy may allow credentialed requests from 3rd party websites |

0 comments on commit 374eec5

Please sign in to comment.