github / github/codeql

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

Abierto
#22,215 1 comentario 0 reacciones 0 asignados Ver en GitHub
false-positive
Lenguaje dominante
CodeQL
Estrellas
10.1k
Forks
2.1k
Merge medio
2 d 15 h
PR fusionados (30 d)
141

Descripción

**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();
}
}
```

Guía de contribución

Abrir la guía de contribución

Línea de trabajo

Comience leyendo csharp/ql/src/Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql, especialmente la comprobación de CSRF para todo el proyecto descrita cerca de la línea 74. Rastree cómo se modelan las acciones de los controladores y sus esquemas de autenticación, y después inspeccione la cobertura existente de la consulta. El trabajo debe considerarse terminado cuando las acciones autenticadas mediante navegador sigan comprobándose, mientras que las acciones de API autenticadas mediante bearer en el mismo proyecto no se notifiquen únicamente porque otra acción utiliza validación de CSRF.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
csharp
Área
authentication, security
Tipo de issue
Error
Dificultad
4/5
Tiempo estimado
3-5 días
Estado de actividad
Tranquilo
Claridad
Bastante claro
Aptitud para principiantes
48/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.