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

Ouverte
#22,215 1 commentaire 0 réactions 0 personnes assignées Voir sur GitHub

Personne n'a encore pris cette issue.

Évaluation

Difficulté
4/5
Temps estimé
3-5 jours
Accessibilité débutants
48/100
Type d'issue
Bug
Clarté
Plutôt claire
Activité
Calme
Stack technique
csharp

Piste de recherche

Commencez par lire csharp/ql/src/Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql, en particulier la vérification CSRF à l’échelle du projet décrite vers la ligne 74. Suivez la manière dont les actions des contrôleurs et leurs schémas d’authentification sont modélisés, puis examinez la couverture existante de la requête. Le travail doit être considéré comme terminé lorsque les actions authentifiées par navigateur continuent d’être vérifiées, tandis que les actions d’API authentifiées par bearer dans le même projet ne sont pas signalées uniquement parce qu’une autre action utilise la validation CSRF.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Description

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();
    }
}
Langage dominant
CodeQL
Étoiles
10.1k
Forks
2.1k
Merge moyen
2 j 11 h
PR mergées (30 j)
129

Guide de contribution

Ouvrir le guide de contribution

Par où commencer

  1. Lisez l'issue en entier, puis le guide de contribution du projet.
  2. Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
  3. Forkez le dépôt et travaillez sur une branche.
  4. Ouvrez une pull request qui référence le numéro de l'issue.

Autres issues de github/codeql

Toutes les issues de github/codeql

Issues similaires

Plus d'issues Security

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.