Skip to content

Commit 77ac908

Browse files
committed
chore: sanitize redirect uri params
1 parent d36fbf2 commit 77ac908

2 files changed

Lines changed: 37 additions & 1 deletion

File tree

app/oauth/views/authorize.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,9 @@ def authorize():
154154

155155
if request.form.get("button") == "deny":
156156
LOG.d("User %s denies Client %s", current_user, client)
157-
final_redirect_uri = f"{redirect_uri}?error=deny&state={state}"
157+
final_redirect_uri = f"{redirect_uri}?error=deny"
158+
if state:
159+
final_redirect_uri += f"&state={encode_url(state)}"
158160
return redirect(final_redirect_uri)
159161

160162
LOG.d("User %s allows Client %s", current_user, client)

tests/oauth/test_authorize.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -780,6 +780,40 @@ def test_authorize_code_id_token_flow(flask_client):
780780
assert verify_id_token(r.json["id_token"])
781781

782782

783+
def test_authorize_deny_escapes_state_param(flask_client):
784+
"""state must be URL-encoded on the deny path to prevent parameter injection"""
785+
user = login(flask_client)
786+
client = Client.create_new("test client", user.id)
787+
Session.commit()
788+
789+
uri = generate_random_uri()
790+
RedirectUri.create(
791+
client_id=client.id,
792+
uri=uri,
793+
commit=True,
794+
)
795+
796+
r = flask_client.post(
797+
url_for(
798+
"oauth.authorize",
799+
client_id=client.oauth_client_id,
800+
state="malicious&injected=param#fragment",
801+
redirect_uri=uri,
802+
response_type="code",
803+
),
804+
data={"button": "deny"},
805+
)
806+
807+
assert r.status_code == 302
808+
o = urlparse(r.location)
809+
queries = parse_qs(o.query)
810+
assert queries["error"] == ["deny"]
811+
# state must round-trip exactly; no extra params or fragment may be injected
812+
assert queries["state"] == ["malicious&injected=param#fragment"]
813+
assert "injected" not in queries
814+
assert o.fragment == ""
815+
816+
783817
def test_authorize_page_invalid_client_id(flask_client):
784818
"""make sure to redirect user to redirect_url?error=invalid_client_id"""
785819
user = login(flask_client)

0 commit comments

Comments
 (0)