Skip to content

Commit 05dc4e0

Browse files
nictukumdrokon-dev
authored andcommitted
fix(security): HTML-escape OAuth callback error param (GHSA-6v2r-2m4r-768m) (pipeboard-co#160)
The local OAuth callback server interpolated the `error` query parameter straight into an HTML error page via an f-string. Any page in the browser could navigate to http://localhost:8080-8089/callback?error=<script>... during the 180s authorization window and execute script on the localhost origin. - html.escape() the error value and cap it at 200 chars before reflecting it - Serve every callback response with Content-Security-Policy default-src 'none', X-Content-Type-Options, X-Frame-Options, Referrer-Policy and Cache-Control; the success page's inline window.close() script runs under a per-response nonce rather than loosening the policy - Drop the unreferenced /token endpoint, which handed the stored OAuth auth code to any same-origin page reachable via the XSS above - Drop the now-unused `quote`/`json` imports Also fixes the success branch, which always 500'd: asyncio.get_event_loop() raises in the callback server's worker thread because no loop is ever set there, so the auth code was never stored. Use time.monotonic() instead. Reported by zx (@manus-pi).
1 parent 099a3b9 commit 05dc4e0

2 files changed

Lines changed: 128 additions & 37 deletions

File tree

‎meta_ads_mcp/core/callback_server.py‎

Lines changed: 36 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,14 @@
22

33
import threading
44
import socket
5-
import asyncio
6-
import json
5+
import time
76
import logging
7+
import secrets
88
import webbrowser
99
import os
10+
from html import escape as html_escape
1011
from http.server import HTTPServer, BaseHTTPRequestHandler
11-
from urllib.parse import urlparse, parse_qs, quote
12+
from urllib.parse import urlparse, parse_qs
1213
from typing import Dict, Any, Optional
1314

1415
from .utils import logger
@@ -27,6 +28,9 @@
2728
# Timeout in seconds before shutting down the callback server
2829
CALLBACK_SERVER_TIMEOUT = 180 # 3 minutes timeout
2930

31+
# Maximum number of characters of an OAuth error echoed back to the browser
32+
MAX_ERROR_DISPLAY_LENGTH = 200
33+
3034

3135
class CallbackHandler(BaseHTTPRequestHandler):
3236
def do_GET(self):
@@ -36,8 +40,6 @@ def do_GET(self):
3640

3741
if self.path.startswith("/callback"):
3842
self._handle_oauth_callback()
39-
elif self.path.startswith("/token"):
40-
self._handle_token()
4143
else:
4244
# If no matching path, return a 404 error
4345
self.send_response(404)
@@ -47,6 +49,20 @@ def do_GET(self):
4749
self.send_response(500)
4850
self.end_headers()
4951

52+
def _send_html(self, html: str, csp: str = "default-src 'none'") -> None:
53+
"""Send an HTML response with hardening headers."""
54+
body = html.encode("utf-8")
55+
self.send_response(200)
56+
self.send_header("Content-type", "text/html; charset=utf-8")
57+
self.send_header("Content-Length", str(len(body)))
58+
self.send_header("Content-Security-Policy", csp)
59+
self.send_header("X-Content-Type-Options", "nosniff")
60+
self.send_header("X-Frame-Options", "DENY")
61+
self.send_header("Referrer-Policy", "no-referrer")
62+
self.send_header("Cache-Control", "no-store")
63+
self.end_headers()
64+
self.wfile.write(body)
65+
5066
def _handle_oauth_callback(self):
5167
"""Handle OAuth callback after user authorization"""
5268
# Check if we're being redirected from Facebook with an authorization code
@@ -58,19 +74,19 @@ def _handle_oauth_callback(self):
5874
state = params.get('state', [None])[0]
5975
error = params.get('error', [None])[0]
6076

61-
# Send 200 OK response with a simple HTML page
62-
self.send_response(200)
63-
self.send_header("Content-type", "text/html")
64-
self.end_headers()
77+
# Anything reaching this endpoint is attacker-controllable, so the error
78+
# is HTML-escaped (and truncated) before it is echoed back to the browser.
79+
csp = "default-src 'none'"
6580

6681
if error:
6782
# User denied access or other error occurred
83+
safe_error = html_escape(error[:MAX_ERROR_DISPLAY_LENGTH])
6884
html = f"""
6985
<html>
7086
<head><title>Authorization Failed</title></head>
7187
<body>
7288
<h1>Authorization Failed</h1>
73-
<p>Error: {error}</p>
89+
<p>Error: {safe_error}</p>
7490
<p>The authorization was cancelled or failed. You can close this window.</p>
7591
</body>
7692
</html>
@@ -85,21 +101,25 @@ def _handle_oauth_callback(self):
85101
token_container.update({
86102
"auth_code": code,
87103
"state": state,
88-
"timestamp": asyncio.get_event_loop().time()
104+
"timestamp": time.monotonic()
89105
})
90106

91-
html = """
107+
# The success page needs its own inline script, so allow just that
108+
# one script via a per-response nonce rather than loosening the CSP.
109+
nonce = secrets.token_urlsafe(16)
110+
csp = f"default-src 'none'; script-src 'nonce-{nonce}'"
111+
html = f"""
92112
<html>
93113
<head><title>Authorization Successful</title></head>
94114
<body>
95115
<h1>✅ Authorization Successful!</h1>
96116
<p>You have successfully authorized the Meta Ads MCP application.</p>
97117
<p>You can now close this window and return to your application.</p>
98-
<script>
118+
<script nonce="{nonce}">
99119
// Try to close the window automatically after 2 seconds
100-
setTimeout(function() {
120+
setTimeout(function() {{
101121
window.close();
102-
}, 2000);
122+
}}, 2000);
103123
</script>
104124
</body>
105125
</html>
@@ -118,27 +138,7 @@ def _handle_oauth_callback(self):
118138
"""
119139
logger.warning("OAuth callback received without code or error")
120140

121-
self.wfile.write(html.encode())
122-
123-
def _handle_token(self):
124-
"""Handle token endpoint for retrieving stored token data"""
125-
# This endpoint allows other parts of the application to retrieve
126-
# token information from the callback server
127-
128-
self.send_response(200)
129-
self.send_header("Content-type", "application/json")
130-
self.end_headers()
131-
132-
# Return current token container contents
133-
response_data = {
134-
"status": "success",
135-
"data": token_container
136-
}
137-
138-
self.wfile.write(json.dumps(response_data).encode())
139-
140-
# The actual token processing is now handled by the auth module
141-
# that imports this module and accesses token_container
141+
self._send_html(html, csp)
142142

143143
# Silence server logs
144144
def log_message(self, format, *args):
@@ -222,7 +222,6 @@ def start_callback_server() -> int:
222222
callback_server_thread.start()
223223

224224
# Wait a moment for the server to start
225-
import time
226225
time.sleep(0.5)
227226

228227
if not callback_server_running:

‎tests/test_callback_server_xss.py‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
"""Regression tests for GHSA-6v2r-2m4r-768m.
2+
3+
The OAuth callback server reflected the `error` query parameter into an HTML
4+
error page with no output encoding, so any page in the browser could navigate
5+
to http://localhost:8080-8089/callback?error=<script>... during the 180s
6+
authorization window and run script on the localhost origin.
7+
"""
8+
9+
import threading
10+
from http.server import HTTPServer
11+
from urllib.parse import quote
12+
13+
import httpx
14+
import pytest
15+
16+
from meta_ads_mcp.core.callback_server import (
17+
CallbackHandler,
18+
MAX_ERROR_DISPLAY_LENGTH,
19+
)
20+
21+
XSS_PAYLOAD = "<script>alert(document.domain)</script>"
22+
23+
24+
@pytest.fixture
25+
def callback_url():
26+
"""Run a real CallbackHandler on an ephemeral localhost port."""
27+
server = HTTPServer(("localhost", 0), CallbackHandler)
28+
thread = threading.Thread(target=server.serve_forever, daemon=True)
29+
thread.start()
30+
try:
31+
yield f"http://localhost:{server.server_port}"
32+
finally:
33+
server.shutdown()
34+
server.server_close()
35+
thread.join(timeout=5)
36+
37+
38+
def test_error_parameter_is_html_escaped(callback_url):
39+
response = httpx.get(f"{callback_url}/callback?error={quote(XSS_PAYLOAD)}")
40+
41+
assert response.status_code == 200
42+
assert XSS_PAYLOAD not in response.text
43+
assert "<script>" not in response.text
44+
# The error is still shown to the user, just entity-encoded.
45+
assert "&lt;script&gt;" in response.text
46+
assert "Authorization Failed" in response.text
47+
48+
49+
def test_error_parameter_cannot_break_out_of_attributes(callback_url):
50+
payload = '" onload="alert(1)'
51+
response = httpx.get(f"{callback_url}/callback?error={quote(payload)}")
52+
53+
assert 'onload="alert(1)' not in response.text
54+
assert "&quot;" in response.text
55+
56+
57+
def test_error_parameter_is_truncated(callback_url):
58+
payload = "A" * (MAX_ERROR_DISPLAY_LENGTH + 500)
59+
response = httpx.get(f"{callback_url}/callback?error={payload}")
60+
61+
assert "A" * MAX_ERROR_DISPLAY_LENGTH in response.text
62+
assert "A" * (MAX_ERROR_DISPLAY_LENGTH + 1) not in response.text
63+
64+
65+
def test_error_page_sets_locked_down_security_headers(callback_url):
66+
response = httpx.get(f"{callback_url}/callback?error=access_denied")
67+
68+
assert response.headers["content-security-policy"] == "default-src 'none'"
69+
assert response.headers["x-content-type-options"] == "nosniff"
70+
assert response.headers["referrer-policy"] == "no-referrer"
71+
72+
73+
def test_success_page_allows_only_its_own_nonced_script(callback_url):
74+
response = httpx.get(f"{callback_url}/callback?code=test-auth-code&state=xyz")
75+
76+
csp = response.headers["content-security-policy"]
77+
assert csp.startswith("default-src 'none'; script-src 'nonce-")
78+
nonce = csp.split("'nonce-")[1].rstrip("'")
79+
# The inline script carries the nonce, so an injected script would not run.
80+
assert f'<script nonce="{nonce}">' in response.text
81+
assert response.headers["x-content-type-options"] == "nosniff"
82+
83+
84+
def test_token_endpoint_no_longer_exposes_the_auth_code(callback_url):
85+
"""/token was unreferenced and handed the stored auth code to any
86+
same-origin page reachable via the XSS above."""
87+
httpx.get(f"{callback_url}/callback?code=test-auth-code&state=xyz")
88+
89+
response = httpx.get(f"{callback_url}/token")
90+
91+
assert response.status_code == 404
92+
assert "test-auth-code" not in response.text

0 commit comments

Comments
 (0)