Skip to content

Commit 05bef12

Browse files
authored
Merge pull request #21265 from github/redsun82/csharp-csrf-inheritance
C#: Fix CSRF query to check antiforgery attributes on base classes
2 parents 1df3adf + f79bd3f commit 05bef12

File tree

6 files changed

+70
-2
lines changed

6 files changed

+70
-2
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,12 @@ predicate hasGlobalAntiForgeryFilter() {
5454
predicate isUnvalidatedPostMethod(Class c, Method m) {
5555
c.(Controller).getAPostActionMethod() = m and
5656
not m.getAnAttribute() instanceof ValidateAntiForgeryTokenAttribute and
57-
not c.getAnAttribute() instanceof ValidateAntiForgeryTokenAttribute
57+
not c.getABaseType*().getAnAttribute() instanceof ValidateAntiForgeryTokenAttribute
5858
or
5959
c.(AspNetCore::MicrosoftAspNetCoreMvcController).getAnActionMethod() = m and
6060
m.getAnAttribute() instanceof AspNetCore::MicrosoftAspNetCoreMvcHttpPostAttribute and
6161
not m.getAnAttribute() instanceof AspNetCore::ValidateAntiForgeryAttribute and
62-
not c.getAnAttribute() instanceof AspNetCore::ValidateAntiForgeryAttribute
62+
not c.getABaseType*().getAnAttribute() instanceof AspNetCore::ValidateAntiForgeryAttribute
6363
}
6464

6565
Element getAValidatedElement() {
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: fix
3+
---
4+
* The `cs/web/missing-token-validation` ("Missing cross-site request forgery token validation") query now recognizes antiforgery attributes on base controller classes, fixing false positives when `[ValidateAntiForgeryToken]` or `[AutoValidateAntiforgeryToken]` is applied to a parent class.

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,34 @@ public void UtilityMethod()
2929
{
3030
}
3131
}
32+
33+
// GOOD: Base class has AutoValidateAntiforgeryToken attribute
34+
[AutoValidateAntiforgeryToken]
35+
public abstract class BaseController : Controller
36+
{
37+
}
38+
39+
public class DerivedController : BaseController
40+
{
41+
// GOOD: Inherits antiforgery validation from base class
42+
[HttpPost]
43+
public ActionResult InheritedValidation()
44+
{
45+
return View();
46+
}
47+
}
48+
49+
// BAD: Base class without antiforgery attribute
50+
public abstract class UnprotectedBaseController : Controller
51+
{
52+
}
53+
54+
public class DerivedUnprotectedController : UnprotectedBaseController
55+
{
56+
// BAD: No antiforgery validation on this or any base class
57+
[HttpPost]
58+
public ActionResult NoInheritedValidation()
59+
{
60+
return View();
61+
}
62+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
| 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. |

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,34 @@ public void UtilityMethod()
2929
{
3030
}
3131
}
32+
33+
// GOOD: Base class has ValidateAntiForgeryToken attribute
34+
[ValidateAntiForgeryToken]
35+
public abstract class BaseController : Controller
36+
{
37+
}
38+
39+
public class DerivedController : BaseController
40+
{
41+
// GOOD: Inherits antiforgery validation from base class
42+
[HttpPost]
43+
public ActionResult InheritedValidation()
44+
{
45+
return View();
46+
}
47+
}
48+
49+
// BAD: Base class without antiforgery attribute
50+
public abstract class UnprotectedBaseController : Controller
51+
{
52+
}
53+
54+
public class DerivedUnprotectedController : UnprotectedBaseController
55+
{
56+
// BAD: No antiforgery validation on this or any base class
57+
[HttpPost]
58+
public ActionResult NoInheritedValidation()
59+
{
60+
return View();
61+
}
62+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
| 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. |

0 commit comments

Comments
 (0)