False positive: "Missing cross-site request forgery token validation" should not apply to Web API Controller Actions sharing a project with Browser-based actions
- Vorherrschende Sprache
- CodeQL
- Sterne
- 10.1k
- Forks
- 2.1k
- Ø Merge
- 2 T. 15 Std.
- Gemergte PRs (30 T.)
- 141
Beschreibung
**Description of the false positive**
* ASP.NET (and ASP.NET Core) projects can have two sets of endpoint handlers (controller actions) which separately handle...
1. Requests originating from web-browsers, which are authenticated using browser cookies or browser-managed HTTP Basic/Digest authentication; including XHR/`fetch`-based requests, as well as ordinary document navigation. These are the kinds of requests that are vulnerable to CSRF attacks and so should use a CSRF validation token or other approach.
2. Requests originating from non-browser-based clients (e.g. daemon processes; cron jobs running curl, etc); these are authenticated using HTTP `Authorization` header (e.g. Bearer tokens). It is not possible for a CSRF attack to succeed in this case (see https://security.stackexchange.com/questions/170388/do-i-need-csrf-token-if-im-using-bearer-jwt ).
Assuming that [**this code**](https://github.com/github/codeql/blob/5690ec71b0efb11fca5e5566b13e1d848b6d0381/csharp/ql/src/Security%20Features/CWE-352/MissingAntiForgeryTokenValidation.ql) is the actual CodeQL analysis rule for this alert (`CWE-352/MissingAntiForgeryTokenValidation.ql`), then the problem is...
1. The rule is only activated if the project uses CSRF _at least once_, anywhere (see [the comment](https://github.com/github/codeql/blob/5690ec71b0efb11fca5e5566b13e1d848b6d0381/csharp/ql/src/Security%20Features/CWE-352/MissingAntiForgeryTokenValidation.ql#L74) where it says _"Verify that validate anti forgery token attributes are used somewhere within this project"_).
2. So it assumes that if _at least one_ controller-action in a project uses CSRF, then **all** controller-actions in the same project _should_ also use CSRF...
* This assumption is incorrect: as mentioned above, it's possible for a project to serve both browser-based requests and non-browser requests - with entirely different authentication schemes and policies such that non-browser-based endpoint-actions cannot be invoked in a browser-based CSRF scenario.
**Code samples or links to source code**
If the two controller-classes are built in a single project, then the fact `BrowserAjaxController` uses `[ValidateAntiForgeryToken]` will cause `MissingAntiForgeryTokenValidation.q` to think that `WebServiceController` should also use `[ValidateAntiForgeryToken]` even though it doesn't use browser-cookies based authentication (due to the different Scheme value).
```
class BrowserAjaxController : Controller
{
[HttpPost("/ajax/exec-rm-rf-root" )]
[Authorize( AuthenticationSchemes = MySchemeNames.BrowserCookiesScheme, Policy = "SomePolicy1" )]
[ValidateAntiForgeryToken]
public IActionResult DoTheThing()
{
return this.Ok();
}
}
class WebServiceController : Controller
{
[HttpPost("/api/arbitrary-operation" )]
[Authorize( AuthenticationSchemes = MySchemeNames.BearerTokenScheme, Policy = "SomePolicy2" )]
public IActionResult DoTheOtherThing()
{
return this.Ok();
}
}
```
Beitragsleitfaden
Rechercherichtung
Start by reading csharp/ql/src/Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql, especially the project-wide CSRF check described near line 74. Trace how controller actions and their authentication schemes are modeled, then inspect the query's existing coverage. Done should mean browser-authenticated actions remain checked while bearer-authenticated API actions in the same project are not reported solely because another action uses CSRF validation.
Vom Indexierungsmodell aus dem Issue-Text verfasst.
Bewertung
- Tech-Stack
- csharp
- Bereich
- authentication, security
- Issue-Typ
- Bug
- Schwierigkeit
- 4/5
- Geschätzter Aufwand
- 3-5 Tage
- Aktivitätsstatus
- Ruhig
- Klarheit
- Größtenteils klar
- Anfängerfreundlichkeit
- 48/100