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

[aegilops]

trafficstars

New version of the existing cs/web/missing-token-validation query that adds:

1. support for AspNetCore
2. lower tolerance for false negatives

Any POST method without either an explicit CSRF attribute, or an identifable global CSRF filter, will be reported as an alert.

This is in contrast to the existing approach, which only alerts if one method is missing a CSRF filter, where others are not.

The existing approach is quite permissive, and results in missing results in both synthetic web applications and in real applications.

[github-actions[bot]]

QHelp previews:

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

Missing cross-site request forgery token validation

Web applications that use tokens to prevent cross-site request forgery (CSRF) should validate the tokens for all HTTP POST requests.

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.

Recommendation

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 [ValidateAntiForgeryToken] attribute.

Alternatively, you may consider including a global filter that applies token validation to all POST requests.

Example

In the following example an ASP.NET MVC Controller is using the [ValidateAntiForgeryToken] attribute to mitigate against CSRF attacks. It has been applied correctly to the UpdateDetails method. However, this attribute has not been applied to the Login method. This should be fixed by adding this attribute.

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();
    }
}

References

• Wikipedia: Cross-Site Request Forgery.
• Microsoft Docs: XSRF/CSRF Prevention in ASP.NET MVC and Web Pages.
• Common Weakness Enumeration: CWE-352.

[aegilops]

I should expand this to cover other definitely state-changing HTTP verbs, such as DELETE and PUT - I think I kept it at just POST to reflect the existing one.

This will not cover CSRF with GET requests, since that would require detecting state-changing GET requests, which requires analysis of the side-effects of the method that handles the request, and is much more involved.

[michaelnebel]

@aegilops : Is the query not needed?