github / github/codeql

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

Đang mở
#3,722 2 bình luận 1 reaction 0 người được giao Xem trên GitHub
false-positive Python
Ngôn ngữ chính
CodeQL
Star
10.1k
Fork
2.1k
Merge trung bình
2 ngày 15 giờ
Pull request đã merge (30 ngày)
141

Mô tả

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

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.