Skip to content

Commit ef991e5

Browse files
authored
Merge pull request #20983 from smowton/smowton/feature/csharp-csrf-aspnetcore
C# CSRF query: add support for ASP.NET Core
2 parents cd6429a + 79718b6 commit ef991e5

File tree

6 files changed

+62
-6
lines changed

6 files changed

+62
-6
lines changed

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

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import csharp
1515
import semmle.code.csharp.frameworks.system.Web
1616
import semmle.code.csharp.frameworks.system.web.Helpers
1717
import semmle.code.csharp.frameworks.system.web.Mvc
18+
import semmle.code.csharp.frameworks.microsoft.AspNetCore as AspNetCore
1819

1920
private Method getAValidatingMethod() {
2021
result = any(AntiForgeryClass a).getValidateMethod()
@@ -35,6 +36,8 @@ private Method getAStartedMethod() {
3536

3637
/**
3738
* 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.
3841
*/
3942
predicate hasGlobalAntiForgeryFilter() {
4043
// A global filter added
@@ -48,16 +51,30 @@ predicate hasGlobalAntiForgeryFilter() {
4851
)
4952
}
5053

51-
from Controller c, Method postMethod
54+
predicate isUnvalidatedPostMethod(Class c, Method m) {
55+
c.(Controller).getAPostActionMethod() = m and
56+
not m.getAnAttribute() instanceof ValidateAntiForgeryTokenAttribute and
57+
not c.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.getAnAttribute() instanceof AspNetCore::ValidateAntiForgeryAttribute
63+
}
64+
65+
Element getAValidatedElement() {
66+
any(ValidateAntiForgeryTokenAttribute a).getTarget() = result
67+
or
68+
any(AspNetCore::ValidateAntiForgeryAttribute a).getTarget() = result
69+
}
70+
71+
from Class c, Method postMethod
5272
where
53-
postMethod = c.getAPostActionMethod() and
54-
// The method is not protected by a validate anti forgery token attribute
55-
not postMethod.getAnAttribute() instanceof ValidateAntiForgeryTokenAttribute and
56-
not c.getAnAttribute() instanceof ValidateAntiForgeryTokenAttribute and
73+
isUnvalidatedPostMethod(c, postMethod) and
5774
// Verify that validate anti forgery token attributes are used somewhere within this project, to
5875
// avoid reporting false positives on projects that use an alternative approach to mitigate CSRF
5976
// issues.
60-
exists(ValidateAntiForgeryTokenAttribute a, Element e | e = a.getTarget()) and
77+
exists(getAValidatedElement()) and
6178
// Also ignore cases where a global anti forgery filter is in use.
6279
not hasGlobalAntiForgeryFilter()
6380
select postMethod,
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 `Missing cross-site request forgery token validation` query was extended to support ASP.NET Core.
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
using Microsoft.AspNetCore.Mvc;
2+
3+
public class HomeController : Controller
4+
{
5+
// BAD: Anti forgery token has been forgotten
6+
[HttpPost]
7+
public ActionResult Login()
8+
{
9+
return View();
10+
}
11+
12+
// GOOD: Anti forgery token is validated
13+
[HttpPost]
14+
[ValidateAntiForgeryToken]
15+
public ActionResult UpdateDetails()
16+
{
17+
return View();
18+
}
19+
20+
// No validation required, as this is a GET method.
21+
public ActionResult ShowHelp()
22+
{
23+
return View();
24+
}
25+
26+
// Should be ignored, because it is not an action method
27+
[NonAction]
28+
public void UtilityMethod()
29+
{
30+
}
31+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
| MissingAntiForgeryTokenValidation.cs:7:25:7:29 | Login | Method 'Login' handles a POST request without performing CSRF token validation. |
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
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

0 commit comments

Comments
 (0)