github / github/codeql

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

Ouverte
#3,722 2 commentaires 1 réaction 0 personnes assignées Voir sur GitHub
false-positive Python
Langage dominant
CodeQL
Étoiles
10.1k
Forks
2.1k
Merge moyen
2 j 15 h
PR mergées (30 j)
141

Description

**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

Guide de contribution

Ouvrir le guide de contribution

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

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