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 個 reaction 已指派 0 人 在 GitHub 檢視

還沒有人認領這個 Issue。

評估

難度
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 天 10 小時
30 天內合併 PR
134

貢獻指南

開啟貢獻指南

從這裡開始

  1. 先讀完整個 Issue,再讀專案的貢獻指南。
  2. 在 Issue 下留言說明你要接手 —— 這能避免兩個人做同樣的事。
  3. Fork 儲存庫,在一個分支上完成修改。
  4. 送出 Pull Request,並在描述裡引用這個 Issue 編號。

github/codeql 的其他 Issue

查看 github/codeql 的全部 Issue

相似的 Issue

更多 Security Issue

把新 issue 寄到你的電子郵件信箱

精選適合新手參與的 GitHub issue 摘要。