Skip to content

Commit e4cf8d6

Browse files
theinfosecguytheinfosecguy
committed
Refactor antiforgery validation query
1 parent db708d4 commit e4cf8d6

1 file changed

Lines changed: 45 additions & 28 deletions

File tree

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

Lines changed: 45 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ private predicate hasAspNetCoreAntiForgeryMiddleware() {
7575
)
7676
}
7777

78-
bindingset[method]
7978
private RequireAntiforgeryTokenAttribute getEffectiveRequireAntiforgeryTokenAttributeOnMethod(
8079
Method method
8180
) {
@@ -90,7 +89,6 @@ private RequireAntiforgeryTokenAttribute getEffectiveRequireAntiforgeryTokenAttr
9089
)
9190
}
9291

93-
bindingset[controller]
9492
private RequireAntiforgeryTokenAttribute getEffectiveRequireAntiforgeryTokenAttributeOnClass(
9593
Class controller
9694
) {
@@ -105,34 +103,53 @@ private RequireAntiforgeryTokenAttribute getEffectiveRequireAntiforgeryTokenAttr
105103
)
106104
}
107105

108-
bindingset[controller, method]
109-
private RequireAntiforgeryTokenAttribute getEffectiveRequireAntiforgeryTokenAttribute(
110-
Class controller, Method method
111-
) {
112-
result = getEffectiveRequireAntiforgeryTokenAttributeOnMethod(method)
113-
or
114-
not exists(getEffectiveRequireAntiforgeryTokenAttributeOnMethod(method)) and
115-
result = getEffectiveRequireAntiforgeryTokenAttributeOnClass(controller)
106+
class MvcControllerPostMethod extends Method {
107+
private Controller controller;
108+
109+
MvcControllerPostMethod() { controller.getAPostActionMethod() = this }
110+
111+
predicate hasValidateAntiForgeryAttribute() {
112+
this.getAnAttribute() instanceof ValidateAntiForgeryTokenAttribute or
113+
controller.getABaseType*().getAnAttribute() instanceof ValidateAntiForgeryTokenAttribute
114+
}
116115
}
117116

118-
bindingset[controller, method]
119-
private predicate hasAspNetCoreAntiForgeryValidation(Class controller, Method method) {
120-
method.getAnAttribute() instanceof AspNetCore::ValidateAntiForgeryAttribute
121-
or
122-
controller.getABaseType*().getAnAttribute() instanceof AspNetCore::ValidateAntiForgeryAttribute
123-
or
124-
hasAspNetCoreAntiForgeryMiddleware() and
125-
getEffectiveRequireAntiforgeryTokenAttribute(controller, method).requiresValidation()
117+
class AspNetCoreControllerPostMethod extends Method {
118+
private AspNetCore::MicrosoftAspNetCoreMvcController controller;
119+
120+
AspNetCoreControllerPostMethod() {
121+
controller.getAnActionMethod() = this and
122+
this.getAnAttribute() instanceof AspNetCore::MicrosoftAspNetCoreMvcHttpPostAttribute
123+
}
124+
125+
predicate hasValidateAntiForgeryAttribute() {
126+
this.getAnAttribute() instanceof AspNetCore::ValidateAntiForgeryAttribute or
127+
controller.getABaseType*().getAnAttribute() instanceof AspNetCore::ValidateAntiForgeryAttribute
128+
}
129+
130+
predicate hasRequireAntiForgeryAttribute() {
131+
hasAspNetCoreAntiForgeryMiddleware() and
132+
(
133+
getEffectiveRequireAntiforgeryTokenAttributeOnMethod(this).requiresValidation()
134+
or
135+
not exists(getEffectiveRequireAntiforgeryTokenAttributeOnMethod(this)) and
136+
getEffectiveRequireAntiforgeryTokenAttributeOnClass(controller).requiresValidation()
137+
)
138+
}
126139
}
127140

128-
predicate isUnvalidatedPostMethod(Class c, Method m) {
129-
c.(Controller).getAPostActionMethod() = m and
130-
not m.getAnAttribute() instanceof ValidateAntiForgeryTokenAttribute and
131-
not c.getABaseType*().getAnAttribute() instanceof ValidateAntiForgeryTokenAttribute
132-
or
133-
c.(AspNetCore::MicrosoftAspNetCoreMvcController).getAnActionMethod() = m and
134-
m.getAnAttribute() instanceof AspNetCore::MicrosoftAspNetCoreMvcHttpPostAttribute and
135-
not hasAspNetCoreAntiForgeryValidation(c, m)
141+
predicate isUnvalidatedAspNetCorePostMethod(AspNetCoreControllerPostMethod m) {
142+
not m.hasValidateAntiForgeryAttribute() and
143+
not m.hasRequireAntiForgeryAttribute()
144+
}
145+
146+
predicate isUnvalidatedMvcPostMethod(MvcControllerPostMethod m) {
147+
not m.hasValidateAntiForgeryAttribute()
148+
}
149+
150+
predicate isUnvalidatedPostMethod(Method m) {
151+
isUnvalidatedMvcPostMethod(m) or
152+
isUnvalidatedAspNetCorePostMethod(m)
136153
}
137154

138155
Element getAValidatedElement() {
@@ -144,9 +161,9 @@ Element getAValidatedElement() {
144161
any(RequireAntiforgeryTokenAttribute a | a.requiresValidation()).getTarget() = result
145162
}
146163

147-
from Class c, Method postMethod
164+
from Method postMethod
148165
where
149-
isUnvalidatedPostMethod(c, postMethod) and
166+
isUnvalidatedPostMethod(postMethod) and
150167
// Verify that validate anti forgery token attributes are used somewhere within this project, to
151168
// avoid reporting false positives on projects that use an alternative approach to mitigate CSRF
152169
// issues.

0 commit comments

Comments
 (0)