fix: close rejected HTTP writes before reuse
This commit is contained in:
1 parent
e037e1b1bd
commit
20f6a68626
3 files changed
+65
-1
No files matched your search
@@ -5871,6 +5871,9 @@ def _csrf_rejection_error(handler) -> str:
|
||||
def _check_csrf(handler) -> bool:
|
||||
"""Reject cross-origin or tokenless authenticated browser unsafe requests."""
|
||||
if not _check_same_origin_browser_request(handler):
|
||||
# CSRF checks run before read_body(). Close rather than reusing an
|
||||
# HTTP/1.1 connection whose unread body would corrupt the next request.
|
||||
handler.close_connection = True
|
||||
return False
|
||||
if not _is_browser_unsafe_request(handler):
|
||||
return True # non-browser clients (curl, MCP, agent) have no Origin/Referer
|
||||
@@ -5883,6 +5886,7 @@ def _check_csrf(handler) -> bool:
|
||||
submitted = handler.headers.get(CSRF_HEADER_NAME) or handler.headers.get("X-CSRF-Token")
|
||||
if verify_csrf_token(cookie_val or "", submitted or ""):
|
||||
return True
|
||||
handler.close_connection = True
|
||||
return _set_csrf_failure_reason(handler, "token_mismatch")
|
||||
|
||||
|
||||
|
||||
@@ -404,7 +404,12 @@ class Handler(BaseHTTPRequestHandler):
|
||||
_is_csp_report_post = (
|
||||
parsed.path == "/api/csp-report" and self.command == "POST"
|
||||
)
|
||||
if not _is_csp_report_post and not check_auth(self, parsed): return
|
||||
if not _is_csp_report_post and not check_auth(self, parsed):
|
||||
# Authentication can reject an unsafe request before its body is
|
||||
# consumed. Reusing that HTTP/1.1 connection would make the
|
||||
# unread body prefix the next request line.
|
||||
self.close_connection = True
|
||||
return
|
||||
result = route_func(self, parsed)
|
||||
if result is False:
|
||||
return j(self, {'error': 'not found'}, status=404)
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
"""Regression tests for HTTP/1.1 write rejection framing.
|
||||
|
||||
A rejected POST can happen before its request body is consumed (authentication
|
||||
or CSRF failure). Keeping that connection alive leaves the body bytes in
|
||||
``rfile``; BaseHTTPRequestHandler then parses them as the next request method.
|
||||
The connection must close after those early rejections.
|
||||
"""
|
||||
from types import SimpleNamespace
|
||||
from typing import Any, cast
|
||||
|
||||
|
||||
def test_auth_rejected_write_closes_connection(monkeypatch):
|
||||
import server
|
||||
|
||||
handler = SimpleNamespace(
|
||||
path="/api/session/new",
|
||||
command="POST",
|
||||
close_connection=False,
|
||||
)
|
||||
monkeypatch.setattr(server, "reset_trusted_auth_request_state", lambda _handler: None)
|
||||
monkeypatch.setattr(server, "get_profile_cookie", lambda _handler: None)
|
||||
monkeypatch.setattr(server, "clear_request_profile", lambda: None)
|
||||
monkeypatch.setattr(server, "check_auth", lambda _handler, _parsed: False)
|
||||
|
||||
def route_must_not_run(_handler, _parsed):
|
||||
raise AssertionError("route ran after auth rejection")
|
||||
|
||||
server.Handler._handle_write(cast(Any, handler), route_must_not_run)
|
||||
|
||||
assert handler.close_connection is True
|
||||
|
||||
|
||||
def test_origin_rejected_csrf_closes_connection(monkeypatch):
|
||||
import api.routes as routes
|
||||
|
||||
handler = SimpleNamespace(close_connection=False)
|
||||
monkeypatch.setattr(routes, "_check_same_origin_browser_request", lambda _handler: False)
|
||||
|
||||
assert routes._check_csrf(handler) is False
|
||||
assert handler.close_connection is True
|
||||
|
||||
|
||||
def test_token_rejected_csrf_closes_connection(monkeypatch):
|
||||
import api.auth as auth
|
||||
import api.routes as routes
|
||||
|
||||
handler = SimpleNamespace(headers={}, close_connection=False)
|
||||
monkeypatch.setattr(routes, "_check_same_origin_browser_request", lambda _handler: True)
|
||||
monkeypatch.setattr(routes, "_is_browser_unsafe_request", lambda _handler: True)
|
||||
monkeypatch.setattr(auth, "is_auth_enabled", lambda: True)
|
||||
monkeypatch.setattr(auth, "parse_cookie", lambda _handler: "session-cookie")
|
||||
monkeypatch.setattr(auth, "verify_csrf_token", lambda _cookie, _token: False)
|
||||
|
||||
assert routes._check_csrf(handler) is False
|
||||
assert handler.close_connection is True
|
||||
Reference in new issue
Block a user