From 9c4af2315829e7514b23becd8a24f5083cfaf9b7 Mon Sep 17 00:00:00 2001 From: Hugo Date: Sat, 5 Sep 2026 14:48:13 +0200 Subject: [PATCH 1/2] test: define redirect sinks as reading their target only --- tests/test_redirect_targets.py | 79 ++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 tests/test_redirect_targets.py diff --git a/tests/test_redirect_targets.py b/tests/test_redirect_targets.py new file mode 100644 index 0000000..3597e0f --- /dev/null +++ b/tests/test_redirect_targets.py @@ -0,0 +1,79 @@ +"""Acceptance tests for redirect sinks reading their target only (issue #71). + +Django's ``redirect("url-name", arg)`` resolves a URL pattern with ``arg`` as a route +parameter; ``HttpResponseRedirect(to)`` and Flask's ``redirect(location, code)`` take the +target first too. Only the first argument is a redirect target, so attacker-controlled +data in the other arguments is not an open redirect: 58 of the 77 ``open-redirect`` +findings on healthchecks were of that shape. + +Expected to remain red until the redirect sinks restrict ``REDIRECT`` to argument 0. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from coretrace_python import engine +from coretrace_python.findings import Finding +from coretrace_python.semantic.symbols import SymbolId +from coretrace_python.source import SourceManager +from coretrace_python.taint import TaintKind + +REPO = Path(__file__).resolve().parent.parent +PLUGINS = REPO / "src" / "coretrace_python" / "bundled" + + +def sinks() -> dict[str, tuple[tuple[TaintKind, tuple[int, ...]], ...]]: + manager = engine.build_manager(engine.build_hir(SourceManager().add_source("e.py", ""))) + loaded = engine.load_plugins([PLUGINS], manager) + table = engine.plugin_models(l.plugin for l in loaded) + names = ("django.shortcuts.redirect", "django.http.HttpResponseRedirect", "flask.redirect") + return {n: table.sink(SymbolId(f"python.{n}")).positions for n in names} # type: ignore[union-attr] + + +MISSING = None if all(sinks().values()) else "redirect sinks have no positions" + + +@pytest.fixture(autouse=True) +def require_positions() -> None: + if MISSING is not None: + pytest.fail(f"redirect targets are not restricted yet: {MISSING}") + + +def check(text: str) -> tuple[Finding, ...]: + return engine.check(SourceManager().add_source("views.py", text), [PLUGINS]) + + +def rules(findings: tuple[Finding, ...]) -> list[str]: + return sorted(f.rule_id for f in findings) + + +DJANGO = "from django.http import HttpRequest\nfrom django.shortcuts import redirect\n\n" + + +def test_redirect_sinks_read_their_first_argument_only() -> None: + for positions in sinks().values(): + assert positions == ((TaintKind.REDIRECT, (0,)),) + + +def test_a_url_name_with_a_tainted_route_argument_is_not_an_open_redirect() -> None: + assert check(DJANGO + "def details(request: HttpRequest, code):\n return redirect('hc-details', code)\n") == () + assert check(DJANGO + "def details(request: HttpRequest):\n return redirect('hc-details', request.GET['code'])\n") == () + + +def test_a_tainted_target_is_still_an_open_redirect() -> None: + assert rules(check(DJANGO + "def go(request: HttpRequest):\n return redirect(request.GET['next'])\n")) == ["open-redirect"] + assert rules( + check( + "from django.http import HttpRequest, HttpResponseRedirect\n\n" + "def go(request: HttpRequest):\n return HttpResponseRedirect(request.GET['next'])\n" + ) + ) == ["open-redirect"] + + +def test_flask_redirect_status_code_is_not_a_target() -> None: + flask = "from flask import Flask, redirect, request\napp = Flask(__name__)\n\n" + assert rules(check(flask + "@app.route('/go')\ndef go():\n return redirect(request.args['next'])\n")) == ["open-redirect"] + assert check(flask + "@app.route('/go')\ndef go():\n return redirect('/home', int(request.args['code']))\n") == () From 2e1fc89dd58c3058096579b4e95838dcb695d531 Mon Sep 17 00:00:00 2001 From: Hugo Date: Sat, 5 Sep 2026 14:48:13 +0200 Subject: [PATCH 2/2] fix: read only the target argument of redirect sinks --- .../bundled/models/django/django_models.py | 11 +- .../bundled/models/flask/flask_models.py | 8 +- tests/regression/expected/DjangoGoat.json | 18 - tests/regression/expected/healthchecks.json | 444 ------------------ 4 files changed, 14 insertions(+), 467 deletions(-) diff --git a/src/coretrace_python/bundled/models/django/django_models.py b/src/coretrace_python/bundled/models/django/django_models.py index b70c70e..e0a55cc 100644 --- a/src/coretrace_python/bundled/models/django/django_models.py +++ b/src/coretrace_python/bundled/models/django/django_models.py @@ -86,6 +86,9 @@ ) +_TARGET_ONLY = ((TaintKind.REDIRECT, (0,)),) + + def _sym(path: str) -> SymbolId: return SymbolId(f"python.{path}") @@ -107,9 +110,11 @@ class DjangoModels(ModelPlugin): Sink(_sym("django.http.response.HttpResponse"), TaintKind.HTML), Sink(_sym("django.template.Template"), TaintKind.HTML), Sink(_sym("django.http.FileResponse"), TaintKind.PATH), - Sink(_sym("django.shortcuts.redirect"), TaintKind.REDIRECT), - Sink(_sym("django.http.HttpResponseRedirect"), TaintKind.REDIRECT), - Sink(_sym("django.http.HttpResponsePermanentRedirect"), TaintKind.REDIRECT), + # Only the target is a redirect: the other arguments of ``redirect`` are route + # parameters resolved through the URL configuration. + Sink(_sym("django.shortcuts.redirect"), TaintKind.REDIRECT, _TARGET_ONLY), + Sink(_sym("django.http.HttpResponseRedirect"), TaintKind.REDIRECT, _TARGET_ONLY), + Sink(_sym("django.http.HttpResponsePermanentRedirect"), TaintKind.REDIRECT, _TARGET_ONLY), Sanitizer(_sym("django.utils.html.escape"), TaintKind.HTML), Sanitizer(_sym("django.utils.html.conditional_escape"), TaintKind.HTML), *(AuthorizationGuard(_sym(decorator), label) for decorator, label in _AUTHORIZATION_DECORATORS), diff --git a/src/coretrace_python/bundled/models/flask/flask_models.py b/src/coretrace_python/bundled/models/flask/flask_models.py index 8073604..765cf20 100644 --- a/src/coretrace_python/bundled/models/flask/flask_models.py +++ b/src/coretrace_python/bundled/models/flask/flask_models.py @@ -23,6 +23,9 @@ ) +_TARGET_ONLY = ((TaintKind.REDIRECT, (0,)),) + + def _sym(path: str) -> SymbolId: return SymbolId(f"python.{path}") @@ -40,8 +43,9 @@ class FlaskModels(ModelPlugin): Sink(_sym("flask.Response"), TaintKind.HTML), Sink(_sym("flask.Markup"), TaintKind.HTML), Sink(_sym("flask.send_file"), TaintKind.PATH), - Sink(_sym("flask.redirect"), TaintKind.REDIRECT), - Sink(_sym("werkzeug.utils.redirect"), TaintKind.REDIRECT), + # ``redirect(location, code)``: the status code is not a target. + Sink(_sym("flask.redirect"), TaintKind.REDIRECT, _TARGET_ONLY), + Sink(_sym("werkzeug.utils.redirect"), TaintKind.REDIRECT, _TARGET_ONLY), Sink(_sym("flask.request.files.save"), TaintKind.PATH), Sanitizer(_sym("werkzeug.utils.secure_filename"), TaintKind.PATH), Sanitizer(_sym("flask.escape"), TaintKind.HTML), diff --git a/tests/regression/expected/DjangoGoat.json b/tests/regression/expected/DjangoGoat.json index c6e1460..bbdfbb5 100644 --- a/tests/regression/expected/DjangoGoat.json +++ b/tests/regression/expected/DjangoGoat.json @@ -6,24 +6,12 @@ "functions_analysed": 39 }, "findings": [ - { - "file": "authentication/views.py", - "line": 33, - "rule": "open-redirect", - "function": "sign_up" - }, { "file": "authentication/views.py", "line": 57, "rule": "sql-injection", "function": "log_in" }, - { - "file": "authentication/views.py", - "line": 82, - "rule": "open-redirect", - "function": "profile_update" - }, { "file": "djangogoat/settings.py", "line": 24, @@ -36,12 +24,6 @@ "rule": "hardcoded-credential", "function": "after_all" }, - { - "file": "notes/views.py", - "line": 86, - "rule": "open-redirect", - "function": "conversation" - }, { "file": "requirements_app.txt", "line": 2, diff --git a/tests/regression/expected/healthchecks.json b/tests/regression/expected/healthchecks.json index 5e94173..65f3618 100644 --- a/tests/regression/expected/healthchecks.json +++ b/tests/regression/expected/healthchecks.json @@ -72,12 +72,6 @@ "rule": "open-redirect", "function": "_redirect_after_login" }, - { - "file": "hc/accounts/views.py", - "line": 120, - "rule": "open-redirect", - "function": "_redirect_after_login" - }, { "file": "hc/accounts/views.py", "line": 142, @@ -114,12 +108,6 @@ "rule": "hardcoded-credential", "function": "profile" }, - { - "file": "hc/accounts/views.py", - "line": 358, - "rule": "open-redirect", - "function": "add_project" - }, { "file": "hc/accounts/views.py", "line": 637, @@ -228,30 +216,12 @@ "rule": "open-redirect", "function": "add_check" }, - { - "file": "hc/front/views.py", - "line": 570, - "rule": "open-redirect", - "function": "update_name" - }, { "file": "hc/front/views.py", "line": 574, "rule": "open-redirect", "function": "update_name" }, - { - "file": "hc/front/views.py", - "line": 599, - "rule": "open-redirect", - "function": "filtering_rules" - }, - { - "file": "hc/front/views.py", - "line": 666, - "rule": "open-redirect", - "function": "update_timeout" - }, { "file": "hc/front/views.py", "line": 670, @@ -264,174 +234,18 @@ "rule": "xss", "function": "ping_body" }, - { - "file": "hc/front/views.py", - "line": 827, - "rule": "open-redirect", - "function": "pause" - }, - { - "file": "hc/front/views.py", - "line": 845, - "rule": "open-redirect", - "function": "pause" - }, - { - "file": "hc/front/views.py", - "line": 863, - "rule": "open-redirect", - "function": "resume" - }, - { - "file": "hc/front/views.py", - "line": 873, - "rule": "open-redirect", - "function": "remove_check" - }, - { - "file": "hc/front/views.py", - "line": 902, - "rule": "open-redirect", - "function": "clear_events" - }, - { - "file": "hc/front/views.py", - "line": 1044, - "rule": "open-redirect", - "function": "uncloak" - }, - { - "file": "hc/front/views.py", - "line": 1068, - "rule": "open-redirect", - "function": "transfer" - }, { "file": "hc/front/views.py", "line": 1115, "rule": "open-redirect", "function": "copy" }, - { - "file": "hc/front/views.py", - "line": 1230, - "rule": "open-redirect", - "function": "channels" - }, - { - "file": "hc/front/views.py", - "line": 1297, - "rule": "open-redirect", - "function": "update_channel_name" - }, - { - "file": "hc/front/views.py", - "line": 1334, - "rule": "open-redirect", - "function": "send_test_notification" - }, - { - "file": "hc/front/views.py", - "line": 1344, - "rule": "open-redirect", - "function": "remove_channel" - }, - { - "file": "hc/front/views.py", - "line": 1353, - "rule": "open-redirect", - "function": "edit_channel" - }, - { - "file": "hc/front/views.py", - "line": 1357, - "rule": "open-redirect", - "function": "edit_channel" - }, - { - "file": "hc/front/views.py", - "line": 1361, - "rule": "open-redirect", - "function": "edit_channel" - }, - { - "file": "hc/front/views.py", - "line": 1365, - "rule": "open-redirect", - "function": "edit_channel" - }, - { - "file": "hc/front/views.py", - "line": 1369, - "rule": "open-redirect", - "function": "edit_channel" - }, - { - "file": "hc/front/views.py", - "line": 1373, - "rule": "open-redirect", - "function": "edit_channel" - }, - { - "file": "hc/front/views.py", - "line": 1377, - "rule": "open-redirect", - "function": "edit_channel" - }, - { - "file": "hc/front/views.py", - "line": 1381, - "rule": "open-redirect", - "function": "edit_channel" - }, - { - "file": "hc/integrations/apprise/views.py", - "line": 31, - "rule": "open-redirect", - "function": "add" - }, - { - "file": "hc/integrations/call/views.py", - "line": 30, - "rule": "open-redirect", - "function": "add" - }, { "file": "hc/integrations/discord/tests/test_add_complete.py", "line": 24, "rule": "hardcoded-credential", "function": "test_it_handles_oauth_response" }, - { - "file": "hc/integrations/discord/views.py", - "line": 57, - "rule": "open-redirect", - "function": "add_complete" - }, - { - "file": "hc/integrations/discord/views.py", - "line": 75, - "rule": "open-redirect", - "function": "add_complete" - }, - { - "file": "hc/integrations/discord/views.py", - "line": 83, - "rule": "open-redirect", - "function": "add_complete" - }, - { - "file": "hc/integrations/discord/views.py", - "line": 90, - "rule": "open-redirect", - "function": "add_complete" - }, - { - "file": "hc/integrations/email/views.py", - "line": 72, - "rule": "open-redirect", - "function": "add" - }, { "file": "hc/integrations/github/tests/test_add_save.py", "line": 20, @@ -468,48 +282,6 @@ "rule": "hardcoded-credential", "function": null }, - { - "file": "hc/integrations/github/views.py", - "line": 61, - "rule": "open-redirect", - "function": "select" - }, - { - "file": "hc/integrations/github/views.py", - "line": 79, - "rule": "open-redirect", - "function": "select" - }, - { - "file": "hc/integrations/github/views.py", - "line": 113, - "rule": "open-redirect", - "function": "save" - }, - { - "file": "hc/integrations/github/views.py", - "line": 113, - "rule": "open-redirect", - "function": "save" - }, - { - "file": "hc/integrations/github/views.py", - "line": 132, - "rule": "open-redirect", - "function": "save" - }, - { - "file": "hc/integrations/github/views.py", - "line": 132, - "rule": "open-redirect", - "function": "save" - }, - { - "file": "hc/integrations/googlechat/views.py", - "line": 33, - "rule": "open-redirect", - "function": "add" - }, { "file": "hc/integrations/gotify/tests/test_edit.py", "line": 18, @@ -540,48 +312,12 @@ "rule": "hardcoded-secret", "function": "test_it_handles_subpath_with_missing_slash" }, - { - "file": "hc/integrations/gotify/views.py", - "line": 26, - "rule": "open-redirect", - "function": "gotify_form" - }, - { - "file": "hc/integrations/gotify/views.py", - "line": 54, - "rule": "open-redirect", - "function": "add" - }, - { - "file": "hc/integrations/group/views.py", - "line": 44, - "rule": "open-redirect", - "function": "add" - }, { "file": "hc/integrations/matrix/tests/test_notify.py", "line": 15, "rule": "hardcoded-credential", "function": null }, - { - "file": "hc/integrations/matrix/views.py", - "line": 37, - "rule": "open-redirect", - "function": "add" - }, - { - "file": "hc/integrations/mattermost/views.py", - "line": 34, - "rule": "open-redirect", - "function": "add" - }, - { - "file": "hc/integrations/msteamsw/views.py", - "line": 29, - "rule": "open-redirect", - "function": "add_msteams" - }, { "file": "hc/integrations/ntfy/tests/test_add.py", "line": 78, @@ -624,42 +360,6 @@ "rule": "hardcoded-credential", "function": "test_uses_default_token_only_on_ntfy_sh" }, - { - "file": "hc/integrations/ntfy/views.py", - "line": 25, - "rule": "open-redirect", - "function": "ntfy_form" - }, - { - "file": "hc/integrations/ntfy/views.py", - "line": 47, - "rule": "open-redirect", - "function": "add" - }, - { - "file": "hc/integrations/opsgenie/views.py", - "line": 31, - "rule": "open-redirect", - "function": "add" - }, - { - "file": "hc/integrations/pagertree/views.py", - "line": 32, - "rule": "open-redirect", - "function": "add" - }, - { - "file": "hc/integrations/pd/views.py", - "line": 52, - "rule": "open-redirect", - "function": "add" - }, - { - "file": "hc/integrations/pd/views.py", - "line": 85, - "rule": "open-redirect", - "function": "add_complete" - }, { "file": "hc/integrations/po/tests/test_notify.py", "line": 15, @@ -678,54 +378,12 @@ "rule": "open-redirect", "function": "add" }, - { - "file": "hc/integrations/po/views.py", - "line": 73, - "rule": "open-redirect", - "function": "add" - }, - { - "file": "hc/integrations/po/views.py", - "line": 85, - "rule": "open-redirect", - "function": "add" - }, { "file": "hc/integrations/pushbullet/tests/test_add_complete.py", "line": 22, "rule": "hardcoded-credential", "function": "test_it_handles_oauth_response" }, - { - "file": "hc/integrations/pushbullet/views.py", - "line": 66, - "rule": "open-redirect", - "function": "add_complete" - }, - { - "file": "hc/integrations/pushbullet/views.py", - "line": 86, - "rule": "open-redirect", - "function": "add_complete" - }, - { - "file": "hc/integrations/pushbullet/views.py", - "line": 93, - "rule": "open-redirect", - "function": "add_complete" - }, - { - "file": "hc/integrations/rocketchat/views.py", - "line": 34, - "rule": "open-redirect", - "function": "add" - }, - { - "file": "hc/integrations/shell/views.py", - "line": 28, - "rule": "open-redirect", - "function": "add" - }, { "file": "hc/integrations/signal/tests/test_notify.py", "line": 273, @@ -744,90 +402,18 @@ "rule": "hardcoded-credential", "function": "test_it_obeys_per_recipient_rate_limit" }, - { - "file": "hc/integrations/signal/views.py", - "line": 34, - "rule": "open-redirect", - "function": "signal_form" - }, - { - "file": "hc/integrations/signal/views.py", - "line": 61, - "rule": "open-redirect", - "function": "add_signal" - }, - { - "file": "hc/integrations/slack/views.py", - "line": 37, - "rule": "open-redirect", - "function": "add" - }, - { - "file": "hc/integrations/slack/views.py", - "line": 93, - "rule": "open-redirect", - "function": "add_complete" - }, - { - "file": "hc/integrations/slack/views.py", - "line": 112, - "rule": "open-redirect", - "function": "add_complete" - }, - { - "file": "hc/integrations/slack/views.py", - "line": 122, - "rule": "open-redirect", - "function": "add_complete" - }, - { - "file": "hc/integrations/sms/views.py", - "line": 29, - "rule": "open-redirect", - "function": "sms_form" - }, - { - "file": "hc/integrations/sms/views.py", - "line": 58, - "rule": "open-redirect", - "function": "add" - }, - { - "file": "hc/integrations/spike/views.py", - "line": 29, - "rule": "open-redirect", - "function": "add_spike" - }, { "file": "hc/integrations/telegram/tests/test_add.py", "line": 13, "rule": "hardcoded-credential", "function": null }, - { - "file": "hc/integrations/telegram/views.py", - "line": 124, - "rule": "open-redirect", - "function": "add" - }, { "file": "hc/integrations/trello/tests/test_notify.py", "line": 36, "rule": "hardcoded-credential", "function": "setUp" }, - { - "file": "hc/integrations/trello/views.py", - "line": 39, - "rule": "open-redirect", - "function": "add" - }, - { - "file": "hc/integrations/victorops/views.py", - "line": 29, - "rule": "open-redirect", - "function": "add" - }, { "file": "hc/integrations/webhook/tests/test_add.py", "line": 79, @@ -840,30 +426,6 @@ "rule": "hardcoded-secret", "function": "test_it_accepts_good_urls" }, - { - "file": "hc/integrations/webhook/views.py", - "line": 30, - "rule": "open-redirect", - "function": "webhook_form" - }, - { - "file": "hc/integrations/webhook/views.py", - "line": 58, - "rule": "open-redirect", - "function": "add" - }, - { - "file": "hc/integrations/whatsapp/views.py", - "line": 28, - "rule": "open-redirect", - "function": "whatsapp_form" - }, - { - "file": "hc/integrations/whatsapp/views.py", - "line": 56, - "rule": "open-redirect", - "function": "add" - }, { "file": "hc/integrations/zulip/tests/test_add.py", "line": 12, @@ -881,12 +443,6 @@ "line": 133, "rule": "hardcoded-credential", "function": "test_it_uses_site_parameter" - }, - { - "file": "hc/integrations/zulip/views.py", - "line": 29, - "rule": "open-redirect", - "function": "add" } ] }