Skip to content

Commit db708d4

Browse files
theinfosecguytheinfosecguy
committed
Recognize RequireAntiforgeryToken attributes
1 parent 08547cb commit db708d4

12 files changed

Lines changed: 318 additions & 4 deletions

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

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,21 +51,97 @@ predicate hasGlobalAntiForgeryFilter() {
5151
)
5252
}
5353

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+
bindingset[method]
79+
private RequireAntiforgeryTokenAttribute getEffectiveRequireAntiforgeryTokenAttributeOnMethod(
80+
Method method
81+
) {
82+
exists(Method attributedMethod |
83+
attributedMethod = method.getOverridee*() and
84+
result = attributedMethod.getAnAttribute() and
85+
not exists(Method closerMethod |
86+
closerMethod = method.getOverridee*() and
87+
closerMethod.getOverridee+() = attributedMethod and
88+
closerMethod.getAnAttribute() instanceof RequireAntiforgeryTokenAttribute
89+
)
90+
)
91+
}
92+
93+
bindingset[controller]
94+
private RequireAntiforgeryTokenAttribute getEffectiveRequireAntiforgeryTokenAttributeOnClass(
95+
Class controller
96+
) {
97+
exists(Class attributedClass |
98+
attributedClass = controller.getBaseClass*() and
99+
result = attributedClass.getAnAttribute() and
100+
not exists(Class closerClass |
101+
closerClass = controller.getBaseClass*() and
102+
closerClass.getBaseClass+() = attributedClass and
103+
closerClass.getAnAttribute() instanceof RequireAntiforgeryTokenAttribute
104+
)
105+
)
106+
}
107+
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)
116+
}
117+
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()
126+
}
127+
54128
predicate isUnvalidatedPostMethod(Class c, Method m) {
55129
c.(Controller).getAPostActionMethod() = m and
56130
not m.getAnAttribute() instanceof ValidateAntiForgeryTokenAttribute and
57131
not c.getABaseType*().getAnAttribute() instanceof ValidateAntiForgeryTokenAttribute
58132
or
59133
c.(AspNetCore::MicrosoftAspNetCoreMvcController).getAnActionMethod() = m and
60134
m.getAnAttribute() instanceof AspNetCore::MicrosoftAspNetCoreMvcHttpPostAttribute and
61-
not m.getAnAttribute() instanceof AspNetCore::ValidateAntiForgeryAttribute and
62-
not c.getABaseType*().getAnAttribute() instanceof AspNetCore::ValidateAntiForgeryAttribute
135+
not hasAspNetCoreAntiForgeryValidation(c, m)
63136
}
64137

65138
Element getAValidatedElement() {
66139
any(ValidateAntiForgeryTokenAttribute a).getTarget() = result
67140
or
68141
any(AspNetCore::ValidateAntiForgeryAttribute a).getTarget() = result
142+
or
143+
hasAspNetCoreAntiForgeryMiddleware() and
144+
any(RequireAntiforgeryTokenAttribute a | a.requiresValidation()).getTarget() = result
69145
}
70146

71147
from Class c, Method postMethod

‎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. |

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
using Microsoft.AspNetCore.Antiforgery;
2+
using Microsoft.AspNetCore.Mvc;
3+
4+
public class HomeController : Controller
5+
{
6+
[HttpPost]
7+
[ValidateAntiForgeryToken]
8+
public ActionResult FilterValidated()
9+
{
10+
return View();
11+
}
12+
13+
[HttpPost]
14+
[RequireAntiforgeryToken]
15+
public ActionResult MetadataWithoutMiddleware() // $ Alert
16+
{
17+
return View();
18+
}
19+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
| MissingAntiForgeryTokenValidation.cs:15:25:15:49 | MetadataWithoutMiddleware | Method 'MetadataWithoutMiddleware' handles a POST request without performing CSRF token validation. |

0 commit comments

Comments
 (0)