github / github/codeql

False positive: Confusing "... may be at an arbitrary position in the sanitized URL."

Abierto
#3,722 2 comentarios 1 reacción 0 asignados Ver en GitHub
false-positive Python
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**

To be honest I ~~don't~~ didn't really understand the aim of the rule at all - but it is reporting code like:

- `assert 'http://www.example.com/' in web_history`
- `assert 'https://github.com' in bookmarks.marks`
- `assert 'https://python.org' in web_history_populated`
- `assert 'http://example.com' in bm.marks`

All of those are checking whether a string (which happens to be an URL) is in a set or list. This doesn't seem to be related to sanitization at all...

Looking at the [query](https://github.com/github/codeql/blob/master/python/ql/src/Security/CWE-020/IncompleteUrlSubstringSanitization.ql) and [help](https://github.com/github/codeql/blob/master/python/ql/src/Security/CWE-020/IncompleteUrlSubstringSanitization.qhelp) I think flagging things like the above isn't the intention of the rule? The help says:

> Sanitizing untrusted URLs is an important technique for
> preventing attacks such as request forgeries and malicious
> redirections. Usually, this is done by checking that the host of a URL
> is in a set of allowed hosts.
>
> However, it is notoriously error-prone to treat the URL as
> a string and check if one of the allowed hosts is a substring of the
> URL. Malicious URLs can bypass such security checks by embedding one
> of the allowed hosts in an unexpected location.
>
> Even if the substring check is not used in a
> security-critical context, the incomplete check may still cause
> undesirable behaviors when the check succeeds accidentally.

So I assume what the query actually indends to check is something like `if 'https://example.com' in url` - but then the RHS must be a string, which is not the case here.

cc @markshannon who added the rule in #820.

**URL to the alert on the project page on LGTM.com**

I'm using CodeQL via GitHub's security scanning (which is in Beta) - so I hope I'm reporting this in the right place, given that the rules behind the two sites seem to be the same...

Here are the alerts I think are false-positives: https://github.com/qutebrowser/qutebrowser/security/code-scanning?query=tool%3ACodeQL+is%3Aclosed+id%3Apy%2Fincomplete-url-substring-sanitization

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.