False positive: "Missing cross-site request forgery token validation" should not apply to Web API Controller Actions sharing a project with Browser-based actions

オープン
#22,215 コメント 1 件 リアクション 0 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

評価

難易度
4/5
見積もり時間
3〜5日
初心者へのやさしさ
48/100
issue の種類
バグ
明瞭さ
おおむね明確
活発さ
静か
技術スタック
csharp

調査の方向性

まず csharp/ql/src/Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql を読み、特に 74 行目付近で説明されているプロジェクト全体の CSRF チェックを確認します。コントローラーのアクションとそれらの認証スキームがどのようにモデル化されているかを追跡し、その後、クエリの既存のカバレッジを調べます。完了の条件は、ブラウザー認証されたアクションのチェックが維持される一方で、同じプロジェクト内の bearer 認証された API アクションが、別のアクションで CSRF 検証が使用されているという理由だけでは報告されないことです。

索引モデルが issue の本文から書いたものです。

説明

false-positive

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 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 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();
    }
}
主要言語
CodeQL
スター
10.1k
フォーク
2.1k
平均マージ
2日 11時間
マージ済み PR(30日)
129

コントリビューションガイド

コントリビューションガイドを開く

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

github/codeql のほかの issue

github/codeql の issue をすべて見る

似ている issue

Security の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。