False positive: "Missing cross-site request forgery token validation" should not apply to Web API Controller Actions sharing a project with Browser-based actions
Nobody has claimed this yet.
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 48/100
- Issue type
- Bug
- Clarity
- Mostly clear
- Activity status
- Quiet
- Tech stack
- csharp
- Domain
- authentication, security
Research direction
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.
Written by the indexing model from the issue text.
Description
Description of the false positive
- ASP.NET (and ASP.NET Core) projects can have two sets of endpoint handlers (controller actions) which separately handle...
- 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. - Requests originating from non-browser-based clients (e.g. daemon processes; cron jobs running curl, etc); these are authenticated using HTTP
Authorizationheader (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 is the actual CodeQL analysis rule for this alert (CWE-352/MissingAntiForgeryTokenValidation.ql), then the problem is...
- The rule is only activated if the project uses CSRF at least once, anywhere (see the comment where it says "Verify that validate anti forgery token attributes are used somewhere within this project").
- 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();
}
}
- Dominant language
- CodeQL
- Stars
- 10.1k
- Forks
- 2.1k
- Avg merge
- 2d 16h
- Merged PRs (30d)
- 143
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from github/codeql
-
agentic-workflows
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
-
false-positive javascript
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
-
C#: cs/simplifiable-boolean-expression false positive on Nullable<bool> compared with a literal Open
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
-
false-positive
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
punkpeye/mcp-remote#369 ·
-
Mend: dependency security vulnerability untriaged
Difficulty 1/5 Under an hour Newbie friendliness 86/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
-
bug
Difficulty 1/5 Under an hour Newbie friendliness 90/100
cisagov/vulnrichment#337 ·
-
bug DUP Reservations
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
bcgov/reserve-rec-public#896 ·