Skip to content

Commit 2107cd2

Browse files
committed
C#: Make the global validation filter compilation aware.
1 parent 9177cce commit 2107cd2

1 file changed

Lines changed: 25 additions & 17 deletions

File tree

csharp/ql/src/Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
*/
1313

1414
import csharp
15+
import semmle.code.csharp.commons.Compilation
1516
import semmle.code.csharp.frameworks.system.Web
1617
import semmle.code.csharp.frameworks.system.web.Helpers
1718
import semmle.code.csharp.frameworks.system.web.Mvc
@@ -34,20 +35,19 @@ private Method getAStartedMethod() {
3435
getAStartedMethod().calls(result)
3536
}
3637

37-
/**
38-
* Holds if the project has a global anti forgery filter.
39-
*/
40-
predicate hasGlobalAntiForgeryFilter() {
41-
// A global filter added
38+
private predicate hasGlobalWebMvcAntiforgeryFilter(Compilation compilation) {
4239
exists(MethodCall addGlobalFilter |
4340
// addGlobalFilter adds a filter to the global filter collection
4441
addGlobalFilter.getTarget() = any(GlobalFilterCollection gfc).getAddMethod() and
4542
// The filter is an antiforgery filter
4643
addGlobalFilter.getArgumentForName("filter").getType() instanceof AntiForgeryAuthorizationFilter and
4744
// The filter is added by the Application_Start() method
48-
getAStartedMethod() = addGlobalFilter.getEnclosingCallable()
45+
getAStartedMethod() = addGlobalFilter.getEnclosingCallable() and
46+
addGlobalFilter.getTarget().getFile() = compilation.getAFileCompiled()
4947
)
50-
or
48+
}
49+
50+
predicate hasGlobalAspNetMvcAntiForgeryFilter(Compilation compilation) {
5151
exists(MethodCall addGlobalFilter, MethodCall registrationCall |
5252
addGlobalFilter.getTarget() =
5353
any(AspNetCore::MicrosoftAspNetCoreMvcFilterCollection collection).getAddMethod() and
@@ -57,7 +57,8 @@ predicate hasGlobalAntiForgeryFilter() {
5757
// The filter is added in an ASP.NET Core registration call, which is provided as a lambda argument
5858
// to the Mvc registration method.
5959
registrationCall.getTarget() instanceof AspNetCore::MicrosoftAspNetCoreMvcRegistration and
60-
registrationCall.getAnArgument() = addGlobalFilter.getEnclosingCallable()
60+
registrationCall.getAnArgument() = addGlobalFilter.getEnclosingCallable() and
61+
addGlobalFilter.getTarget().getFile() = compilation.getAFileCompiled()
6162
)
6263
}
6364

@@ -77,11 +78,12 @@ private class RequireAntiforgeryTokenAttribute extends Attribute {
7778
}
7879
}
7980

80-
private predicate hasAspNetCoreAntiForgeryMiddleware() {
81+
private predicate hasAspNetCoreAntiForgeryMiddleware(Compilation compilation) {
8182
exists(MethodCall call |
8283
call.getTarget()
8384
.hasFullyQualifiedName("Microsoft.AspNetCore.Builder",
84-
"AntiforgeryApplicationBuilderExtensions", "UseAntiforgery")
85+
"AntiforgeryApplicationBuilderExtensions", "UseAntiforgery") and
86+
call.getFile() = compilation.getAFileCompiled()
8587
)
8688
}
8789

@@ -116,7 +118,12 @@ private RequireAntiforgeryTokenAttribute getEffectiveRequireAntiforgeryTokenAttr
116118
class MvcControllerPostMethod extends Method {
117119
private Controller controller;
118120

119-
MvcControllerPostMethod() { controller.getAPostActionMethod() = this }
121+
MvcControllerPostMethod() {
122+
controller.getAPostActionMethod() = this and
123+
exists(Compilation compilation | compilation.getAFileCompiled() = this.getFile() |
124+
not hasGlobalWebMvcAntiforgeryFilter(compilation)
125+
)
126+
}
120127

121128
predicate hasValidateAntiForgeryAttribute() {
122129
this.getAnAttribute() instanceof ValidateAntiForgeryTokenAttribute or
@@ -126,10 +133,13 @@ class MvcControllerPostMethod extends Method {
126133

127134
class AspNetCoreControllerPostMethod extends Method {
128135
private AspNetCore::MicrosoftAspNetCoreMvcController controller;
136+
private Compilation compilation;
129137

130138
AspNetCoreControllerPostMethod() {
131139
controller.getAnActionMethod() = this and
132-
this.getAnAttribute() instanceof AspNetCore::MicrosoftAspNetCoreMvcHttpPostAttribute
140+
this.getAnAttribute() instanceof AspNetCore::MicrosoftAspNetCoreMvcHttpPostAttribute and
141+
compilation.getAFileCompiled() = this.getFile() and
142+
not hasGlobalAspNetMvcAntiForgeryFilter(compilation)
133143
}
134144

135145
predicate hasValidateAntiForgeryAttribute() {
@@ -138,7 +148,7 @@ class AspNetCoreControllerPostMethod extends Method {
138148
}
139149

140150
predicate hasRequireAntiForgeryAttribute() {
141-
hasAspNetCoreAntiForgeryMiddleware() and
151+
hasAspNetCoreAntiForgeryMiddleware(compilation) and
142152
(
143153
getEffectiveRequireAntiforgeryTokenAttributeOnMethod(this).requiresValidation()
144154
or
@@ -167,7 +177,7 @@ Element getAValidatedElement() {
167177
or
168178
any(AspNetCore::ValidateAntiForgeryAttribute a).getTarget() = result
169179
or
170-
hasAspNetCoreAntiForgeryMiddleware() and
180+
hasAspNetCoreAntiForgeryMiddleware(_) and
171181
any(RequireAntiforgeryTokenAttribute a | a.requiresValidation()).getTarget() = result
172182
}
173183

@@ -177,9 +187,7 @@ where
177187
// Verify that validate anti forgery token attributes are used somewhere within this project, to
178188
// avoid reporting false positives on projects that use an alternative approach to mitigate CSRF
179189
// issues.
180-
exists(getAValidatedElement()) and
181-
// Also ignore cases where a global anti forgery filter is in use.
182-
not hasGlobalAntiForgeryFilter()
190+
exists(getAValidatedElement())
183191
select postMethod,
184192
"Method '" + postMethod.getName() +
185193
"' handles a POST request without performing CSRF token validation."

0 commit comments

Comments
 (0)