Skip to content

Commit 374eec5

Browse files
committed
Second round feedback
1 parent 2a4ed45 commit 374eec5

File tree

6 files changed

+29
-15
lines changed

6 files changed

+29
-15
lines changed

csharp/ql/src/experimental/CWE-942/CorsMisconfiguration.ql

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,9 @@ private predicate hasDangerousOrigins(MethodCall m) {
3030
m.getTarget()
3131
.hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsPolicyBuilder",
3232
"WithOrigins") and
33-
(
34-
m.getAnArgument().getValue() = ["null", "*"]
35-
or
36-
exists(StringLiteral idStr |
37-
idStr.getValue().toLowerCase().matches(["null", "*"]) and
38-
DataFlow::localExprFlow(idStr, m.getAnArgument())
39-
)
33+
exists(StringLiteral idStr |
34+
idStr.getValue().toLowerCase().matches(["null", "*"]) and
35+
TaintTracking::localExprTaint(idStr, m.getAnArgument())
4036
)
4137
}
4238

@@ -45,14 +41,14 @@ where
4541
(
4642
usedPolicy(add_policy) and
4743
// Misconfigured origin affects used policy
48-
add_policy.getArgument(1) = child.getParent*()
44+
getCallableFromExpr(add_policy.getArgument(1)).calls*(child.getTarget())
4945
or
5046
add_policy
5147
.getTarget()
5248
.hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsOptions",
5349
"AddDefaultPolicy") and
5450
// Misconfigured origin affects added default policy
55-
add_policy.getArgument(0) = child.getParent*()
51+
getCallableFromExpr(add_policy.getArgument(0)).calls*(child.getTarget())
5652
) and
5753
(hasDangerousOrigins(child) or allowAnyOrigin(child))
5854
select add_policy, "The following CORS policy may allow requests from 3rd party websites"

csharp/ql/src/experimental/CWE-942/CorsMisconfigurationCredentials.ql

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,16 @@ class AllowsCredentials extends MethodCall {
2626
from MethodCall add_policy, MethodCall setIsOriginAllowed, AllowsCredentials allowsCredentials
2727
where
2828
(
29-
add_policy.getArgument(1) = setIsOriginAllowed.getParent*() and
29+
getCallableFromExpr(add_policy.getArgument(1)).calls*(setIsOriginAllowed.getTarget()) and
3030
usedPolicy(add_policy) and
31-
add_policy.getArgument(1) = allowsCredentials.getParent*()
31+
getCallableFromExpr(add_policy.getArgument(1)).calls*(allowsCredentials.getTarget())
3232
or
3333
add_policy
3434
.getTarget()
3535
.hasFullyQualifiedName("Microsoft.AspNetCore.Cors.Infrastructure.CorsOptions",
3636
"AddDefaultPolicy") and
37-
add_policy.getArgument(0) = setIsOriginAllowed.getParent*() and
38-
add_policy.getArgument(0) = allowsCredentials.getParent*()
37+
getCallableFromExpr(add_policy.getArgument(0)).calls*(setIsOriginAllowed.getTarget()) and
38+
getCallableFromExpr(add_policy.getArgument(0)).calls*(allowsCredentials.getTarget())
3939
) and
4040
setIsOriginAllowedReturnsTrue(setIsOriginAllowed)
4141
select add_policy,

csharp/ql/src/experimental/CWE-942/CorsMisconfigurationLib.qll

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,18 @@
11
import csharp
22
import DataFlow
33

4+
/**
5+
* Gets the actual callable corresponding to the expression `e`.
6+
*/
7+
Callable getCallableFromExpr(Expr e) {
8+
exists(Expr dcArg | dcArg = e.(DelegateCreation).getArgument() |
9+
result = dcArg.(CallableAccess).getTarget() or
10+
result = dcArg.(AnonymousFunctionExpr)
11+
)
12+
or
13+
result = e
14+
}
15+
416
/**
517
* Holds if the `Callable` c throws any exception other than `ThrowsArgumentNullException`
618
*/

csharp/ql/test/experimental/CWE-942/CorsMisconfiguration.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,13 @@ public void ConfigureServices(string[] args) {
1111
builder.Services.AddCors(options => {
1212
options.AddPolicy(MyAllowSpecificOrigins,
1313
policy => {
14-
policy.AllowAnyOrigin().AllowCredentials().AllowAnyHeader().AllowAnyMethod();
14+
policy.AllowAnyOrigin().AllowAnyHeader().AllowAnyMethod();
1515
});
16+
options.AddDefaultPolicy(
17+
builder => builder
18+
.WithOrigins(["*"])
19+
.AllowAnyMethod()
20+
.AllowAnyHeader());
1621
});
1722

1823
var app = builder.Build();
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
| CorsMisconfiguration.cs:12:7:15:10 | call to method AddPolicy | The following CORS policy may allow requests from 3rd party websites |
2+
| CorsMisconfiguration.cs:16:7:20:26 | call to method AddDefaultPolicy | The following CORS policy may allow requests from 3rd party websites |
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
| CorsMiconfigurationCredentials.cs:18:5:22:24 | call to method AddPolicy | The following CORS policy may allow credentialed requests from 3rd party websites |
1+
| CorsMiconfigurationCredentials.cs:12:7:15:10 | call to method AddPolicy | The following CORS policy may allow credentialed requests from 3rd party websites |

0 commit comments

Comments
 (0)