Python: Should an empty `netloc` count as a `py/url-redirection` sanitizer when `///evil.com` passes the check?
I maintainer di solito rispondono entro 1 giorno
Valutazione
- Difficoltà
- 4/5
- Tempo stimato
- 3-5 giorni
- Idoneità per principianti
- 52/100
- Tipo di issue
- Bug
- Chiarezza
- Specificata chiaramente
- Stato di attività
- Attiva
- Stack tecnologico
- python
- Ambito
- security, testing-qa
Direzione di ricerca
Read python/ql/lib/semmle/python/frameworks/Stdlib/Urllib.qll around NetlocCheck, then inspect python/ql/src/Security/CWE-601/UrlRedirect.qhelp, redirect_good2.py, and the ok8 case in python/ql/test/query-tests/Security/CWE-601-UrlRedirect/test.py. Reproduce the three-slash case and run the URL redirect query tests; done means the sanitizer, help example, and test annotations consistently account for the demonstrated bypass.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
Description of the issue
py/url-redirection treats a check that urlparse(target).netloc is empty as a sanitizer, and the query help recommends this check, together with an empty scheme, as a way to validate redirect targets. I'm wondering whether the check is meant to be complete, because for ///evil.com or ////evil.com, urlparse returns an empty scheme and an empty netloc, so the check passes, while browsers resolve the redirect to https://evil.com/.
Affected sanitizer
NetlocCheck treats a read of urlparse(...).netloc used as a condition, or compared with anything, as a barrier guard for the NoBackslashes state. The query help recommends it in redirect_good2.py, and ok8 in the query test annotates it # OK.
Steps to reproduce
The help example, unchanged:
from flask import Flask, request, redirect
from urllib.parse import urlparse
app = Flask(__name__)
@app.route('/')
def hello():
target = request.args.get('target', '')
target = target.replace('\\', '')
if not urlparse(target).netloc and not urlparse(target).scheme:
# relative path, safe to redirect
return redirect(target, code=302)
# ignore the target and redirect to the home page
return redirect('/', code=302)
The check from ok8, plus an unchecked redirect for comparison:
# Same check as `ok8` in python/ql/test/query-tests/Security/CWE-601-UrlRedirect/test.py
from flask import Flask, request, redirect
from urllib.parse import urlparse
app = Flask(__name__)
@app.route('/ok8')
def ok8():
untrusted = request.args.get('target', '')
untrusted = untrusted.replace("\\", "/")
if not urlparse(untrusted).netloc:
return redirect(untrusted, code=302) # OK
return redirect("/", code=302)
@app.route('/bad')
def bad():
return redirect(request.args.get('target', ''), code=302) # control: reported
py/url-redirection (and python-security-extended.qls) reports only /bad. With Flask's test client, the help example produces these Location headers, and ok8 produces the same ones:
target |
Location |
|---|---|
/home |
/home |
https://evil.com |
/ |
//evil.com |
/ |
///evil.com |
///evil.com |
////evil.com |
////evil.com |
new URL("///evil.com", "https://example.com/login") and new URL("////evil.com", "https://example.com/login") both give https://evil.com/ (WHATWG URL parser, as used by browsers).
This looks related to #16041: #16646 added the scheme check to the help example for https:/example.com, but URLs without a scheme that start with three or more slashes still pass. Django's url_has_allowed_host_and_scheme, which the help recommends for Django, rejects them explicitly (django/utils/http.py: "Chrome considers any URL with more than two slashes to be absolute, but urlsplit is not so flexible. Treat any url with three slashes as unsafe.").
The check also occurs in real code: KindleEar#734 used the same netloc/scheme check as the help example (url_for(...) if parts.netloc or parts.scheme else nextUrl), and ////evil.com bypassed it. DeepBI#169 describes the same bypass of a urlsplit scheme/netloc check.
Question
Is NetlocCheck intended to treat scheme-less URLs that start with three or more slashes as safe? Would it be reasonable to keep the taint unless such URLs are also rejected, as Django does, and to update the help example and the ok8 annotation accordingly?
Environment
- CodeQL CLI 2.27.2 (
codeql/python-queries1.8.12) - CodeQL repository commit for the links above:
781e068896eb7a1a104f1378e8f3f68a1074a015(the files are unchanged since 2.27.2) - Python 3.10.12, Flask 3.0.0, Werkzeug 3.0.1; Node.js v24.18.0 for the URL resolution; Linux/amd64
- Lingua principale
- CodeQL
- Stelle
- 10.2k
- Fork
- 2.1k
- Merge medio
- 2g 17h
- PR unite (30g)
- 130
Preparare l'ambiente
Avvia il container di sviluppo del progetto nel browser, con il tuo account GitHub.
- Nessun Dockerfile né file Docker Compose
- Nessun modello di pull request
- Leggi la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di github/codeql
-
JavaScript externs model StorageEvent() with no type argumentForse già presa @asgerf l’ha presa 5 giorni fa. Apertafalse-positive javascript
Difficoltà 2/5 1-3 ore Idoneità per principianti 84/100
github/codeql#22632 · 3 commenti · 1 reazione · 2 assegnatari ·
I maintainer di solito rispondono entro 1 giorno
-
Add AlertSuppression.ql for Rust (inline // codeql[...] suppression)Forse già presa @cnuss l’ha presa 206 giorni fa. Aperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
github/codeql#21637 · 2 commenti ·
I maintainer di solito rispondono entro 1 giorno
-
false-positive
Difficoltà 2/5 1-3 ore Idoneità per principianti 70/100
github/codeql#21076 · 3 commenti · 3 reazioni ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 5/5 Più di una settimana Idoneità per principianti 35/100
github/codeql#22883 · 2 commenti ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 5/5 Più di una settimana Idoneità per principianti 25/100
github/codeql#22882 · 2 commenti ·
I maintainer di solito rispondono entro 1 giorno
Tutte le issue di github/codeql
Issue simili
-
from-review-extraction library privacy python severity:suggested
Difficoltà 2/5 1-3 ore Idoneità per principianti 83/100
LearningCircuit/local-deep-research#7951 ·
I maintainer di solito rispondono entro 2 giorni
-
clawsweeper:fix-shape-clear clawsweeper:queueable-fix clawsweeper:source-repro impact:ux-friction issue-rating: 🦞 diamond lobster no-stale P2
Difficoltà 2/5 1-3 ore Idoneità per principianti 74/100
openclaw/openclaw#183929 · 1 commento · 1 reazione ·
I maintainer di solito rispondono entro 1 giorno
-
Review backlog (security): master-branch remote guard trusts unbound provenance package fieldApertaagent/reviewer
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
projectbluefin/utah-packages#634 ·
I maintainer di solito rispondono entro 1 giorno
-
io
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 73/100
I maintainer di solito rispondono entro 1 giorno