Python: Should an empty `netloc` count as a `py/url-redirection` sanitizer when `///evil.com` passes the check?
维护者通常 1 天内回复
还没有人认领这个 Issue。
评估
- 难度
- 4/5
- 预计耗时
- 3-5 天
- 新手友好度
- 52/100
- Issue 类型
- 缺陷
- 描述清晰度
- 描述清楚
- 活跃度
- 活跃
- 技术栈
- python
- 领域
- security, testing-qa
调研方向
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.
由索引模型根据 Issue 内容生成。
描述
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
- 主要语言
- CodeQL
- 星标
- 10.2k
- 派生
- 2.1k
- 平均合并
- 2 天 18 小时
- 30 天内合并 PR
- 142
环境准备
在浏览器里用你自己的 GitHub 账号启动这个项目的开发容器。
- 没有 Dockerfile 或 Docker Compose 文件
- 没有 Pull Request 模板
- 阅读贡献指南
从这里开始
- 先读完整个 Issue,再读项目的贡献指南。
- 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
- Fork 仓库,在一个分支上完成修改。
- 提交 Pull Request,并在描述里引用这个 Issue 编号。
github/codeql 的其他 Issue
-
false-positive javascript
难度 2/5 1-3 小时 新手友好度 84/100
github/codeql#22632 · 3 条评论 · 1 个 reaction · 已指派 2 人 ·
维护者通常 1 天内回复
-
Add AlertSuppression.ql for Rust (inline // codeql[...] suppression)可能已有人在做 @cnuss 于 202 天前认领。 未关闭
难度 2/5 1-3 小时 新手友好度 78/100
维护者通常 1 天内回复
-
false-positive
难度 2/5 1-3 小时 新手友好度 70/100
github/codeql#21076 · 3 条评论 · 3 个 reaction ·
维护者通常 1 天内回复
-
false-positive
难度 4/5 3-5 天 新手友好度 48/100
维护者通常 1 天内回复
-
question
难度 4/5 3-5 天 新手友好度 45/100
维护者通常 1 天内回复
相似的 Issue
-
bug screener-policy
难度 2/5 1-3 小时 新手友好度 68/100
ditto-assistant/ditto-subnet#3067 ·
维护者通常 1 天内回复
-
Mend: dependency security vulnerability
难度 2/5 1-3 小时 新手友好度 72/100
kube-tarian/sigrun#202 ·
-
/submit, /my-list, and /onboarding return 500 when Supabase isn't configured可能已有人在做 @Aryan-Techie 今天认领。 未关闭
难度 2/5 1-3 小时 新手友好度 75/100
Your-Voldemort/StudentStack#127 · 1 条评论 ·
维护者通常 1 天内回复
-
[🐛 Bug]: OAuth2 Proxy logout leaves the Auth.js session active when switching users可能已有人在做 @juanbailon 今天认领。 未关闭
难度 2/5 1-3 小时 新手友好度 73/100
keephq/keep#6888 · 1 个 reaction ·
维护者通常 1 天内回复
-
docs: four SECURITY.md statements no longer match the code (allowed hosts, body cap, OIDC, ACLs)未关闭
难度 2/5 1-3 小时 新手友好度 78/100
akitaonrails/ai-memory#1561 ·
维护者通常 1 天内回复