Skip to content

Commit 69573f4

Browse files
michaelnebelmichaelnebel
authored
Merge pull request #22322 from theinfosecguy/csharp-require-antiforgery-token
[C#] Recognize RequireAntiforgeryToken attributes
2 parents e5f5e5b + e4cf8d6 commit 69573f4

12 files changed

Lines changed: 344 additions & 13 deletions

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

Lines changed: 104 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -51,26 +51,119 @@ predicate hasGlobalAntiForgeryFilter() {
5151
)
5252
}
5353

54-
predicate isUnvalidatedPostMethod(Class c, Method m) {
55-
c.(Controller).getAPostActionMethod() = m and
56-
not m.getAnAttribute() instanceof ValidateAntiForgeryTokenAttribute and
57-
not c.getABaseType*().getAnAttribute() instanceof ValidateAntiForgeryTokenAttribute
58-
or
59-
c.(AspNetCore::MicrosoftAspNetCoreMvcController).getAnActionMethod() = m and
60-
m.getAnAttribute() instanceof AspNetCore::MicrosoftAspNetCoreMvcHttpPostAttribute and
61-
not m.getAnAttribute() instanceof AspNetCore::ValidateAntiForgeryAttribute and
62-
not c.getABaseType*().getAnAttribute() instanceof AspNetCore::ValidateAntiForgeryAttribute
54+
private class RequireAntiforgeryTokenAttribute extends Attribute {
55+
RequireAntiforgeryTokenAttribute() {
56+
this.getType()
57+
.hasFullyQualifiedName("Microsoft.AspNetCore.Antiforgery",
58+
"RequireAntiforgeryTokenAttribute")
59+
}
60+
61+
predicate requiresValidation() {
62+
not exists(this.getArgument(0))
63+
or
64+
this.getArgument(0).isImplicit()
65+
or
66+
this.getArgument(0).getValue() = "true"
67+
}
68+
}
69+
70+
private predicate hasAspNetCoreAntiForgeryMiddleware() {
71+
exists(MethodCall call |
72+
call.getTarget()
73+
.hasFullyQualifiedName("Microsoft.AspNetCore.Builder",
74+
"AntiforgeryApplicationBuilderExtensions", "UseAntiforgery")
75+
)
76+
}
77+
78+
private RequireAntiforgeryTokenAttribute getEffectiveRequireAntiforgeryTokenAttributeOnMethod(
79+
Method method
80+
) {
81+
exists(Method attributedMethod |
82+
attributedMethod = method.getOverridee*() and
83+
result = attributedMethod.getAnAttribute() and
84+
not exists(Method closerMethod |
85+
closerMethod = method.getOverridee*() and
86+
closerMethod.getOverridee+() = attributedMethod and
87+
closerMethod.getAnAttribute() instanceof RequireAntiforgeryTokenAttribute
88+
)
89+
)
90+
}
91+
92+
private RequireAntiforgeryTokenAttribute getEffectiveRequireAntiforgeryTokenAttributeOnClass(
93+
Class controller
94+
) {
95+
exists(Class attributedClass |
96+
attributedClass = controller.getBaseClass*() and
97+
result = attributedClass.getAnAttribute() and
98+
not exists(Class closerClass |
99+
closerClass = controller.getBaseClass*() and
100+
closerClass.getBaseClass+() = attributedClass and
101+
closerClass.getAnAttribute() instanceof RequireAntiforgeryTokenAttribute
102+
)
103+
)
104+
}
105+
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+
}
115+
}
116+
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+
}
139+
}
140+
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)
63153
}
64154

65155
Element getAValidatedElement() {
66156
any(ValidateAntiForgeryTokenAttribute a).getTarget() = result
67157
or
68158
any(AspNetCore::ValidateAntiForgeryAttribute a).getTarget() = result
159+
or
160+
hasAspNetCoreAntiForgeryMiddleware() and
161+
any(RequireAntiforgeryTokenAttribute a | a.requiresValidation()).getTarget() = result
69162
}
70163

71-
from Class c, Method postMethod
164+
from Method postMethod
72165
where
73-
isUnvalidatedPostMethod(c, postMethod) and
166+
isUnvalidatedPostMethod(postMethod) and
74167
// Verify that validate anti forgery token attributes are used somewhere within this project, to
75168
// avoid reporting false positives on projects that use an alternative approach to mitigate CSRF
76169
// issues.

‎csharp/ql/src/change-notes/2026-08-09-require-antiforgery-token.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The `cs/web/missing-token-validation` query now recognizes enabled ASP.NET Core `RequireAntiforgeryToken` attributes when antiforgery middleware is used.

‎csharp/ql/test/query-tests/Security Features/CWE-352/disabled-require-antiforgery/MissingAntiForgeryTokenValidation.cs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
using Microsoft.AspNetCore.Antiforgery;
2+
using Microsoft.AspNetCore.Builder;
3+
using Microsoft.AspNetCore.Mvc;
4+
5+
public class HomeController : Controller
6+
{
7+
[HttpPost]
8+
[RequireAntiforgeryToken(false)]
9+
public ActionResult DisabledValidation()
10+
{
11+
return View();
12+
}
13+
14+
[HttpPost]
15+
public ActionResult MissingValidation()
16+
{
17+
return View();
18+
}
19+
}
20+
21+
public class Startup
22+
{
23+
public void Configure(IApplicationBuilder app)
24+
{
25+
app.UseAntiforgery();
26+
}
27+
}

‎csharp/ql/test/query-tests/Security Features/CWE-352/disabled-require-antiforgery/MissingAntiForgeryTokenValidation.expected‎

Whitespace-only changes.

‎csharp/ql/test/query-tests/Security Features/CWE-352/disabled-require-antiforgery/MissingAntiForgeryTokenValidation.qlref‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
query: Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql

‎csharp/ql/test/query-tests/Security Features/CWE-352/disabled-require-antiforgery/options‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
semmle-extractor-options: /nostdlib /noconfig
2+
semmle-extractor-options: --load-sources-from-project:${testdir}/../../../../resources/stubs/_frameworks/Microsoft.AspNetCore.App/Microsoft.AspNetCore.App.csproj

‎csharp/ql/test/query-tests/Security Features/CWE-352/missing-aspnetcore/MissingAntiForgeryTokenValidation.cs‎

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
using Microsoft.AspNetCore.Mvc;
2+
using Microsoft.AspNetCore.Antiforgery;
3+
using Microsoft.AspNetCore.Builder;
24

35
public class HomeController : Controller
46
{
7+
private const bool ValidationEnabled = true;
8+
private const bool ValidationDisabled = false;
9+
510
// BAD: Anti forgery token has been forgotten
611
[HttpPost]
712
public ActionResult Login() // $ Alert
@@ -17,6 +22,46 @@ public ActionResult UpdateDetails()
1722
return View();
1823
}
1924

25+
// GOOD: Anti forgery token is required by ASP.NET Core middleware
26+
[HttpPost]
27+
[RequireAntiforgeryToken]
28+
public ActionResult UpdateProfile()
29+
{
30+
return View();
31+
}
32+
33+
// GOOD: Explicitly requires anti forgery validation
34+
[HttpPost]
35+
[RequireAntiforgeryToken(true)]
36+
public ActionResult UpdatePreferences()
37+
{
38+
return View();
39+
}
40+
41+
// GOOD: Named and constant arguments are supported
42+
[HttpPost]
43+
[RequireAntiforgeryToken(required: ValidationEnabled)]
44+
public ActionResult UpdateSettings()
45+
{
46+
return View();
47+
}
48+
49+
// BAD: Explicitly disables anti forgery validation
50+
[HttpPost]
51+
[RequireAntiforgeryToken(false)]
52+
public ActionResult DisabledValidation() // $ Alert
53+
{
54+
return View();
55+
}
56+
57+
// BAD: A false constant also disables anti forgery validation
58+
[HttpPost]
59+
[RequireAntiforgeryToken(ValidationDisabled)]
60+
public ActionResult ConstantDisabledValidation() // $ Alert
61+
{
62+
return View();
63+
}
64+
2065
// No validation required, as this is a GET method.
2166
public ActionResult ShowHelp()
2267
{
@@ -46,6 +91,110 @@ public ActionResult InheritedValidation()
4691
}
4792
}
4893

94+
// GOOD: Base class requires anti forgery validation
95+
[RequireAntiforgeryToken]
96+
public abstract class AntiforgeryBaseController : Controller
97+
{
98+
}
99+
100+
public abstract class IntermediateAntiforgeryController : AntiforgeryBaseController
101+
{
102+
}
103+
104+
public class DerivedAntiforgeryController : IntermediateAntiforgeryController
105+
{
106+
[HttpPost]
107+
public ActionResult InheritedRequiredValidation()
108+
{
109+
return View();
110+
}
111+
}
112+
113+
[RequireAntiforgeryToken]
114+
public class ProtectedController : Controller
115+
{
116+
// GOOD: Controller requires anti forgery validation
117+
[HttpPost]
118+
public ActionResult ProtectedAction()
119+
{
120+
return View();
121+
}
122+
123+
// BAD: Action-level metadata overrides the controller metadata
124+
[HttpPost]
125+
[RequireAntiforgeryToken(false)]
126+
public ActionResult DisabledAction() // $ Alert
127+
{
128+
return View();
129+
}
130+
}
131+
132+
[RequireAntiforgeryToken(false)]
133+
public class DisabledController : Controller
134+
{
135+
// BAD: Controller explicitly disables anti forgery validation
136+
[HttpPost]
137+
public ActionResult DisabledControllerAction() // $ Alert
138+
{
139+
return View();
140+
}
141+
142+
// GOOD: Action-level metadata overrides the controller metadata
143+
[HttpPost]
144+
[RequireAntiforgeryToken(true)]
145+
public ActionResult EnabledAction()
146+
{
147+
return View();
148+
}
149+
}
150+
151+
[RequireAntiforgeryToken]
152+
public abstract class ProtectedBaseController : Controller
153+
{
154+
}
155+
156+
[RequireAntiforgeryToken(false)]
157+
public class DisabledDerivedController : ProtectedBaseController
158+
{
159+
// BAD: Derived controller metadata overrides base controller metadata
160+
[HttpPost]
161+
public ActionResult DisabledInheritedAction() // $ Alert
162+
{
163+
return View();
164+
}
165+
}
166+
167+
[AutoValidateAntiforgeryToken]
168+
public class FilterProtectedController : Controller
169+
{
170+
// GOOD: Disabled middleware metadata does not disable the MVC filter
171+
[HttpPost]
172+
[RequireAntiforgeryToken(false)]
173+
public ActionResult FilterProtectedAction()
174+
{
175+
return View();
176+
}
177+
}
178+
179+
public abstract class MethodMetadataBaseController : Controller
180+
{
181+
[RequireAntiforgeryToken]
182+
public virtual ActionResult InheritedMethodValidation()
183+
{
184+
return View();
185+
}
186+
}
187+
188+
public class MethodMetadataController : MethodMetadataBaseController
189+
{
190+
// GOOD: Method metadata is inherited by the override
191+
[HttpPost]
192+
public override ActionResult InheritedMethodValidation()
193+
{
194+
return View();
195+
}
196+
}
197+
49198
// BAD: Base class without antiforgery attribute
50199
public abstract class UnprotectedBaseController : Controller
51200
{
@@ -60,3 +209,29 @@ public ActionResult NoInheritedValidation() // $ Alert
60209
return View();
61210
}
62211
}
212+
213+
namespace Custom
214+
{
215+
public class RequireAntiforgeryTokenAttribute : System.Attribute
216+
{
217+
}
218+
219+
public class CustomAttributeController : Controller
220+
{
221+
// BAD: An unrelated attribute with the same name does not provide validation
222+
[HttpPost]
223+
[RequireAntiforgeryToken]
224+
public ActionResult LookalikeAttribute() // $ Alert
225+
{
226+
return View();
227+
}
228+
}
229+
}
230+
231+
public class Startup
232+
{
233+
public void Configure(IApplicationBuilder app)
234+
{
235+
app.UseAntiforgery();
236+
}
237+
}

‎csharp/ql/test/query-tests/Security Features/CWE-352/missing-aspnetcore/MissingAntiForgeryTokenValidation.expected‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,8 @@
1-
| MissingAntiForgeryTokenValidation.cs:7:25:7:29 | Login | Method 'Login' handles a POST request without performing CSRF token validation. |
2-
| MissingAntiForgeryTokenValidation.cs:58:25:58:45 | NoInheritedValidation | Method 'NoInheritedValidation' handles a POST request without performing CSRF token validation. |
1+
| MissingAntiForgeryTokenValidation.cs:12:25:12:29 | Login | Method 'Login' handles a POST request without performing CSRF token validation. |
2+
| MissingAntiForgeryTokenValidation.cs:52:25:52:42 | DisabledValidation | Method 'DisabledValidation' handles a POST request without performing CSRF token validation. |
3+
| MissingAntiForgeryTokenValidation.cs:60:25:60:50 | ConstantDisabledValidation | Method 'ConstantDisabledValidation' handles a POST request without performing CSRF token validation. |
4+
| MissingAntiForgeryTokenValidation.cs:126:25:126:38 | DisabledAction | Method 'DisabledAction' handles a POST request without performing CSRF token validation. |
5+
| MissingAntiForgeryTokenValidation.cs:137:25:137:48 | DisabledControllerAction | Method 'DisabledControllerAction' handles a POST request without performing CSRF token validation. |
6+
| MissingAntiForgeryTokenValidation.cs:161:25:161:47 | DisabledInheritedAction | Method 'DisabledInheritedAction' handles a POST request without performing CSRF token validation. |
7+
| MissingAntiForgeryTokenValidation.cs:207:25:207:45 | NoInheritedValidation | Method 'NoInheritedValidation' handles a POST request without performing CSRF token validation. |
8+
| MissingAntiForgeryTokenValidation.cs:224:29:224:46 | LookalikeAttribute | Method 'LookalikeAttribute' handles a POST request without performing CSRF token validation. |

0 commit comments

Comments
 (0)