Missing cross-site request forgery token validation query (experimental)

csharp/ql/lib/experimental/code/csharp/MissingAntiForgeryTokenValidationAspNetCore.qll

@@ -0,0 +1,159 @@
+/** Library to support cs/web/missing-token-validation-aspnetcore query */
+
+private import csharp
+private import semmle.code.csharp.frameworks.system.Web
+private import semmle.code.csharp.frameworks.system.web.Helpers as Helpers
+private import semmle.code.csharp.frameworks.system.web.Mvc as Mvc
+private import semmle.code.csharp.frameworks.microsoft.AspNetCore as AspNetCore
+
+/** Defines classes used for anti-forgery tokens. */
+class AntiForgeryClass extends Class {
+  AntiForgeryClass() {
+    this instanceof Helpers::AntiForgeryClass or
+    this instanceof AspNetCore::AntiForgeryClass
+  }
+
+  /** Gets the `Validate` method. */
+  Method getValidateMethod() {
+    result = this.(Helpers::AntiForgeryClass).getValidateMethod() or
+    result = this.(AspNetCore::AntiForgeryClass).getValidateMethod()
+  }
+}
+
+/** Defines the anti-forgery token attribute. */
+class ValidateAntiForgeryTokenAttribute extends Attribute {
+  ValidateAntiForgeryTokenAttribute() {
+    this instanceof Mvc::ValidateAntiForgeryTokenAttribute or
+    this instanceof AspNetCore::ValidateAntiForgeryAttribute
+  }
+}
+
+/** Defines a generic controller including Mvc and AspNetCore */
+class Controller extends Class {
+  Controller() {
+    this instanceof Mvc::Controller or
+    this instanceof AspNetCore::MicrosoftAspNetCoreMvcController
+  }
+
+  /** Gets a Method with a POST action */
+  Method getAPostActionMethod() {
+    result = this.(Mvc::Controller).getAPostActionMethod()
+    or
+    exists(Method method |
+      method = this.(AspNetCore::MicrosoftAspNetCoreMvcController).getAnActionMethod() and
+      method.getAnAttribute() instanceof AspNetCore::MicrosoftAspNetCoreMvcHttpPostAttribute and
+      result = method
+    )
+  }
+}
+
+/** An `AuthorizationFilter` that calls the `AntiForgery.Validate` method. */
+class AntiForgeryAuthorizationFilter extends Mvc::AuthorizationFilter {
+  AntiForgeryAuthorizationFilter() {
+    this.getOnAuthorizationMethod().calls*(any(AntiForgeryClass a).getValidateMethod())
+  }
+}
+
+/** Find a filter that auto-validates anti-forgery tokens. */
+class AutoValidateAntiForgeryTokenFilter extends Expr {
+  AutoValidateAntiForgeryTokenFilter() {
+    exists(
+      MethodCall addControllers, LambdaExpr lambda, ParameterAccess options, PropertyCall filters,
+      MethodCall add
+    |
+      // "AddMvc", "AddControllersWithViews", "AddMvcCore", "AddControllers", "AddRazorPages", so generalised to "Add*" to future-proof
+      addControllers.getTarget().getName().matches("Add%") and
+      addControllers.getArgument(1) = lambda and
+      lambda.getAParameter().getAnAccess() = options and
+      filters.getQualifier() = options and
+      filters.getTarget().getName() = "get_Filters" and
+      add.getQualifier() = filters and
+      add.getTarget().getUndecoratedName() = "Add" and
+      this = add and
+      (
+        // new AutoValidateAntiforgeryTokenAttribute()
+        exists(ObjectCreation obj |
+          add.getArgument(0) = obj and
+          obj.getType() instanceof AutoValidateAntiforgeryTokenAttributeType
+        )
+        or
+        // typeof(AutoValidateAntiforgeryTokenAttribute)
+        exists(TypeAccess access |
+          add.getArgument(0).(TypeofExpr).getAChild() = access and
+          access.getType() instanceof AutoValidateAntiforgeryTokenAttributeType
+        )
+        or
+        // Add<AutoValidateAntiforgeryTokenAttribute>()
+        add.getTarget().getName() = "Add<AutoValidateAntiforgeryTokenAttribute>"
+      )
+    )
+  }
+}
+
+/** Accounts for custom auto-validation classes with a similar name */
+class AutoValidateAntiforgeryTokenAttributeType extends Type {
+  AutoValidateAntiforgeryTokenAttributeType() {
+    this.getName().matches("%AutoValidateAntiforgeryTokenAttribute")
+  }
+}
+
+/** The ignore anti-forgery token attribute. */
+class IgnoreAntiforgeryTokenAttribute extends Attribute {
+  IgnoreAntiforgeryTokenAttribute() { this.getType().getName() = "IgnoreAntiforgeryTokenAttribute" }
+}
+
+/** The auto-validate anti-forgery token attribute. */
+class AutoValidateAntiforgeryTokenAttribute extends Attribute {
+  AutoValidateAntiforgeryTokenAttribute() {
+    this.getType() instanceof AutoValidateAntiforgeryTokenAttributeType
+  }
+}
+
+/**
+ * Holds if the project has a global anti forgery filter.
+ */
+predicate hasGlobalAntiForgeryFilter() {
+  // A global filter added
+  exists(MethodCall addGlobalFilter |
+    // addGlobalFilter adds a filter to the global filter collection
+    addGlobalFilter.getTarget() = any(Mvc::GlobalFilterCollection gfc).getAddMethod() and
+    // The filter is an antiforgery filter
+    addGlobalFilter.getArgumentForName("filter").getType() instanceof AntiForgeryAuthorizationFilter and
+    // The filter is added by the Application_Start() method
+    any(WebApplication wa)
+        .getApplication_StartMethod()
+        .calls*(addGlobalFilter.getEnclosingCallable())
+  )
+  or
+  // for ASP.NET Core
+  exists(AutoValidateAntiForgeryTokenFilter filter)
+}
+
+/** Holds if the Method has the name "Login" */
+predicate isLoginAction(Method m) { m.getName() = "Login" }
+
+/** Holds if a Method has a CSRF attribute. */
+predicate methodHasCsrfAttribute(Method method) {
+  exists(Attribute attribute |
+    (
+      attribute instanceof ValidateAntiForgeryTokenAttribute or
+      attribute instanceof IgnoreAntiforgeryTokenAttribute
+    ) and
+    (
+      method.getAnAttribute() = attribute or
+      method.getAnUltimateImplementee().getAnAttribute() = attribute
+    )
+  )
+}
+
+/** Holds if a Controller has a CSRF attribute. */
+predicate controllerHasCsrfAttribute(Controller c) {
+  exists(Attribute attribute |
+    (
+      attribute instanceof ValidateAntiForgeryTokenAttribute or
+      attribute instanceof IgnoreAntiforgeryTokenAttribute or
+      attribute instanceof AutoValidateAntiforgeryTokenAttribute
+    ) and
+    c.getBaseClass*().getAnAttribute() = attribute
+  )
+}

csharp/ql/src/change-notes/2024-07-09-missing-token-validation-aspnetcore.md

@@ -0,0 +1,6 @@
+---
+category: minorAnalysis
+---
+* A "Missing cross-site request forgery token validation" query for AspNetCore (`cs/web/missing-token-validation-aspnetcore`) was added as an `experimental` query.
+  * Unlike the existing "Missing cross-site request forgery token validation" query (`cs/web/missing-token-validation`), this query adds support for AspNetCore
+  * The new query also does not tolerate situations where no CSRF token validation is present, so will produce many more (legitimate) results than the existing query. For this reason alone, it has been published as `experimental`.

csharp/ql/src/experimental/CWE-352/MissingAntiForgeryTokenValidationAspNetCore.cs

@@ -0,0 +1,19 @@
+using System.Web.Mvc;
+
+public class HomeController : Controller
+{
+    // BAD: Anti forgery token has been forgotten
+    [HttpPost]
+    public ActionResult Login()
+    {
+        return View();
+    }
+
+    // GOOD: Anti forgery token is validated
+    [HttpPost]
+    [ValidateAntiForgeryToken]
+    public ActionResult UpdateDetails()
+    {
+        return View();
+    }
+}

csharp/ql/src/experimental/CWE-352/MissingAntiForgeryTokenValidationAspNetCore.qhelp

@@ -0,0 +1,42 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Web applications that use tokens to prevent cross-site request forgery
+(CSRF) should validate the tokens for all HTTP POST requests.</p>
+
+<p>Although login and authentication methods are not vulnerable to traditional
+CSRF attacks, they still need to be protected with a token or other mitigation.
+This because an unprotected login page can be used by an attacker to force a
+login using an account controlled by the attacker. Subsequent requests to the
+site are then made using this account, without the user being aware that this is
+the case. This can result in the user associating private information with the
+attacker-controlled account.</p>
+</overview>
+
+<recommendation>
+<p>The appropriate attribute should be added to this method to ensure the
+anti-forgery token is validated when this action method is called. If using the
+MVC-provided anti-forgery framework this will be the
+<code>[ValidateAntiForgeryToken]</code> attribute.
+</p>
+<p>Alternatively, you may consider including a global filter that applies token
+validation to all POST requests.</p>
+
+</recommendation>
+
+<example>
+<p>In the following example an ASP.NET MVC <code>Controller</code> is using the
+<code>[ValidateAntiForgeryToken]</code> attribute to mitigate against CSRF attacks.
+It has been applied correctly to the <code>UpdateDetails</code> method. However,
+this attribute has not been applied to the <code>Login</code> method. This should
+be fixed by adding this attribute.</p>
+<sample src="MissingAntiForgeryTokenValidationAspNetCore.cs" />
+</example>
+
+<references>
+<li>Wikipedia: <a href="https://en.wikipedia.org/wiki/Cross-site_request_forgery">Cross-Site Request Forgery</a>.</li>
+<li>Microsoft Docs: <a href="https://docs.microsoft.com/en-us/aspnet/mvc/overview/security/xsrfcsrf-prevention-in-aspnet-mvc-and-web-pages">XSRF/CSRF Prevention in ASP.NET MVC and Web Pages</a>.</li>
+</references>
+</qhelp>

csharp/ql/src/experimental/CWE-352/MissingAntiForgeryTokenValidationAspNetCore.ql

@@ -0,0 +1,29 @@
+/**
+ * @name Missing cross-site request forgery token validation
+ * @description Handling a POST request without verifying that the request came from the user
+ *              allows a malicious attacker to submit a request on behalf of the user.
+ * @kind problem
+ * @problem.severity error
+ * @security-severity 8.8
+ * @precision high
+ * @id cs/web/missing-token-validation-aspnetcore
+ * @tags security
+ *       external/cwe/cwe-352
+ *       experimental
+ */
+
+import csharp
+import experimental.code.csharp.MissingAntiForgeryTokenValidationAspNetCore
+
+from Method postMethod, Controller c
+where
+  postMethod = c.getAPostActionMethod() and
+  // The method is not protected by a "validate anti forgery token" attribute (or ignores it)
+  not methodHasCsrfAttribute(postMethod) and
+  // the class of the method doesn't have a "validate anti forgery token" method (or ignores it)
+  not controllerHasCsrfAttribute(c) and
+  // a global anti forgery filter is not in use
+  not hasGlobalAntiForgeryFilter()
+select postMethod,
+  "Method '" + postMethod.getName() +
+    "' handles a POST request without performing CSRF token validation."

csharp/ql/test/experimental/CWE-352/MissingAntiForgeryTokenValidationAspNetCore.cs

@@ -0,0 +1,27 @@
+using System.Web.Mvc;
+
+public class HomeController : Controller
+{
+    // BAD: Anti forgery token has been forgotten
+    [HttpPost]
+    public string Login()
+    {
+        return View();
+    }
+
+    // GOOD: Anti forgery token is validated
+    [HttpPost]
+    [ValidateAntiForgeryToken]
+    public string UpdateDetails()
+    {
+        return View();
+    }
+
+    // Dummy View method to allow compilation
+    protected string View()
+    {
+        // Simulate returning a view. In a real application, this would return an actual view using a subclass of ActionResult.
+        return "This is a dummy view.";
+    }
+}
+

csharp/ql/test/experimental/CWE-352/MissingAntiForgeryTokenValidationAspNetCore.expected

@@ -0,0 +1 @@
+| MissingAntiForgeryTokenValidationAspNetCore.cs:7:19:7:23 | Login | Method 'Login' handles a POST request without performing CSRF token validation. |

csharp/ql/test/experimental/CWE-352/MissingAntiForgeryTokenValidationAspNetCore.qlref

@@ -0,0 +1 @@
+experimental/CWE-352/MissingAntiForgeryTokenValidationAspNetCore.ql

csharp/ql/test/experimental/CWE-352/options

@@ -0,0 +1,4 @@
+semmle-extractor-options: /nostdlib /noconfig
+semmle-extractor-options: --load-sources-from-project:${testdir}/../../resources/stubs/_frameworks/Microsoft.NETCore.App/Microsoft.NETCore.App.csproj
+semmle-extractor-options: --load-sources-from-project:${testdir}/../../resources/stubs/_frameworks/Microsoft.AspNetCore.App/Microsoft.AspNetCore.App.csproj
+semmle-extractor-options: ${testdir}/../../resources/stubs/System.Web.cs