Skip to content

Commit 22002fd

Browse files
committed
C#: Detect possible global auto validate configuration to remove FPs.
1 parent 00ccd64 commit 22002fd

2 files changed

Lines changed: 35 additions & 7 deletions

File tree

csharp/ql/lib/semmle/code/csharp/frameworks/microsoft/AspNetCore.qll

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,15 @@ class ValidateAntiForgeryAttribute extends Attribute {
144144
}
145145
}
146146

147+
/**
148+
* The `Microsoft.AspNetCore.Mvc.AutoValidateAntiforgeryTokenAttribute` class.
149+
*/
150+
class AutoValidateAntiforgeryTokenAttribute extends Class {
151+
AutoValidateAntiforgeryTokenAttribute() {
152+
this.hasFullyQualifiedName("Microsoft.AspNetCore.Mvc", "AutoValidateAntiforgeryTokenAttribute")
153+
}
154+
}
155+
147156
/**
148157
* A class that has a name like `[Auto...]Validate[...]Anti[Ff]orgery[...Token]` and implements `IFilterMetadata` interface
149158
* This class can be added to a collection of global `MvcOptions.Filters` collection.
@@ -230,11 +239,20 @@ private Assembly getAnAssemblyFor(Type type) {
230239
result = getACompilationFor(type).getOutputAssembly()
231240
}
232241

233-
private predicate isMicrosoftAspNetCoreMvcRegistration(MethodCall call) {
234-
call.getTarget()
235-
.hasFullyQualifiedName("Microsoft.Extensions.DependencyInjection",
236-
["MvcServiceCollectionExtensions", "MvcCoreServiceCollectionExtensions"],
237-
["AddControllers", "AddControllersWithViews", "AddMvc", "AddMvcCore"])
242+
/**
243+
* A method that is a registration of an ASP.NET Core MVC service, i.e. `AddControllers`, `AddControllersWithViews`, `AddMvc`, or `AddMvcCore`.
244+
*/
245+
class MicrosoftAspNetCoreMvcRegistration extends Method {
246+
MicrosoftAspNetCoreMvcRegistration() {
247+
this.hasFullyQualifiedName("Microsoft.Extensions.DependencyInjection",
248+
["MvcServiceCollectionExtensions", "MvcCoreServiceCollectionExtensions"],
249+
["AddControllers", "AddControllersWithViews", "AddMvc", "AddMvcCore"])
250+
}
251+
}
252+
253+
/** Holds if the method call is a registration of an ASP.NET Core MVC service. */
254+
predicate isMicrosoftAspNetCoreMvcRegistration(MethodCall call) {
255+
call.getTarget() instanceof MicrosoftAspNetCoreMvcRegistration
238256
}
239257

240258
private predicate isMicrosoftAspNetCoreMvcApplication(Compilation compilation) {

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,6 @@ private Method getAStartedMethod() {
3636

3737
/**
3838
* Holds if the project has a global anti forgery filter.
39-
*
40-
* No AspNetCore case here as the corresponding class doesn't seem to exist.
4139
*/
4240
predicate hasGlobalAntiForgeryFilter() {
4341
// A global filter added
@@ -49,6 +47,18 @@ predicate hasGlobalAntiForgeryFilter() {
4947
// The filter is added by the Application_Start() method
5048
getAStartedMethod() = addGlobalFilter.getEnclosingCallable()
5149
)
50+
or
51+
exists(MethodCall addGlobalFilter, MethodCall registrationCall |
52+
addGlobalFilter.getTarget() =
53+
any(AspNetCore::MicrosoftAspNetCoreMvcFilterCollection collection).getAddMethod() and
54+
// The filter is the `AutoValidateAntiforgeryTokenAttribute` filter.
55+
addGlobalFilter.getArgument(0).getType() instanceof
56+
AspNetCore::AutoValidateAntiforgeryTokenAttribute and
57+
// The filter is added in an ASP.NET Core registration call, which is provided as a lambda argument
58+
// to the Mvc registration method.
59+
registrationCall.getTarget() instanceof AspNetCore::MicrosoftAspNetCoreMvcRegistration and
60+
registrationCall.getAnArgument() = addGlobalFilter.getEnclosingCallable()
61+
)
5262
}
5363

5464
private class RequireAntiforgeryTokenAttribute extends Attribute {

0 commit comments

Comments
 (0)