fix(security): escape OAuth error parameter in callback HTML to prevent reflected XSS
This commit is contained in:
@@ -5,6 +5,7 @@ import stat
|
||||
import sys
|
||||
from io import BytesIO
|
||||
from unittest.mock import patch, MagicMock
|
||||
from urllib.parse import quote
|
||||
|
||||
import pytest
|
||||
|
||||
@@ -498,6 +499,28 @@ class TestCallbackHandlerIsolation:
|
||||
assert result["error"] == "access_denied"
|
||||
|
||||
|
||||
class TestCallbackHandlerErrorEscaping:
|
||||
"""Regression: a hostile ``error`` parameter must be HTML-escaped before
|
||||
being reflected into the callback response body (reflected XSS)."""
|
||||
|
||||
def test_hostile_error_is_escaped_in_response_body(self):
|
||||
HandlerClass, result = _make_callback_handler()
|
||||
|
||||
handler = HandlerClass.__new__(HandlerClass)
|
||||
handler.path = "/callback?error=" + quote("<script>alert(1)</script>")
|
||||
handler.wfile = BytesIO()
|
||||
handler.send_response = MagicMock()
|
||||
handler.send_header = MagicMock()
|
||||
handler.end_headers = MagicMock()
|
||||
handler.do_GET()
|
||||
|
||||
body = handler.wfile.getvalue().decode("utf-8")
|
||||
assert "<script>" not in body
|
||||
assert "<script>alert(1)</script>" in body
|
||||
# The raw (unescaped) value is still captured for programmatic use.
|
||||
assert result["error"] == "<script>alert(1)</script>"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# TOCTOU port reservation (#22161)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -11,6 +11,7 @@ redirect_host, client_name, client_metadata_url, cimd, user_agent, timeout."""
|
||||
import asyncio
|
||||
import contextlib
|
||||
import contextvars
|
||||
import html
|
||||
import importlib.util as _importlib_util
|
||||
import json
|
||||
import logging
|
||||
@@ -473,7 +474,7 @@ def _make_callback_handler() -> tuple[type, dict]:
|
||||
parsed = _parse_redirect_query(urlparse(self.path).query)
|
||||
result.update(auth_code=parsed["code"], state=parsed["state"], error=parsed["error"], iss=parsed["iss"])
|
||||
body = ("<h2>Authorization Successful</h2><p>You can close this tab and return to Hermes.</p>" if parsed["code"]
|
||||
else f"<h2>Authorization Failed</h2><p>Error: {parsed['error'] or 'unknown'}</p>")
|
||||
else f"<h2>Authorization Failed</h2><p>Error: {html.escape(parsed['error'] or 'unknown')}</p>")
|
||||
self.send_response(200)
|
||||
self.send_header("Content-Type", "text/html; charset=utf-8")
|
||||
self.end_headers()
|
||||
|
||||
Reference in New Issue
Block a user