False positive: "Missing cross-site request forgery token validation" should not apply to Web API Controller Actions sharing a project with Browser-based actions
- 主要言語
- CodeQL
- スター
- 10.1k
- フォーク
- 2.1k
- 平均マージ
- 2日 15時間
- マージ済み PR(30日)
- 141
説明
**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();
}
}
```
コントリビューションガイド
調査の方向性
まず csharp/ql/src/Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql を読み、特に 74 行目付近で説明されているプロジェクト全体の CSRF チェックを確認します。コントローラーのアクションとそれらの認証スキームがどのようにモデル化されているかを追跡し、その後、クエリの既存のカバレッジを調べます。完了の条件は、ブラウザー認証されたアクションのチェックが維持される一方で、同じプロジェクト内の bearer 認証された API アクションが、別のアクションで CSRF 検証が使用されているという理由だけでは報告されないことです。
索引モデルが issue の本文から書いたものです。
評価
- 技術スタック
- csharp
- 領域
- authentication, security
- issue の種類
- バグ
- 難易度
- 4/5
- 見積もり時間
- 3〜5日
- 活発さ
- 静か
- 明瞭さ
- おおむね明確
- 初心者へのやさしさ
- 48/100