mirror of
https://github.com/Donkie/Spoolman.git
synced 2026-10-22 01:52:17 -05:00
Donkie
c33f65243c
Closes the two confirmed CSRF findings and the cross-site websocket hijack
from the backend security audit (todo.md tasks 1 and 3).
POST /setting/{key} took a bare `str` body, and FastAPI only JSON-parses
application/*json -- anything else arrived as raw bytes that lax-mode Pydantic
coerced. A text/plain body was therefore accepted, which is exactly what
<form enctype="text/plain"> sends, so any website the user visited could
rewrite base_url, label_designs, locations or extra_fields_* through their
browser. Now requires a JSON content type.
The audit suggested Body(media_type="application/json") for this, but that only
annotates the OpenAPI schema and is not enforced at runtime -- a text/plain post
to a pinned endpoint still returns 200. Hence an explicit dependency. Both
clients already send application/json, and a charset parameter is accepted, so
this is wire-compatible.
TrustedOriginMiddleware refuses state-changing HTTP methods and every websocket
handshake from an untrusted origin, using the helper added in the previous
commit. A middleware rather than per-endpoint dependencies so that none of the
nine websocket endpoints can be missed, and one added later cannot forget to
opt in. Reads are left alone -- the same-origin policy already covers them --
but every websocket is guarded, since websockets are exempt from CORS and
otherwise leak the whole inventory to any origin that opens one.
It is ordered inside the CORS middleware so a legitimate cross-origin client
can read the 403 body instead of an opaque CORS error.
Writing a malformed extra_fields_* array through the settings endpoint bypassed
validate_extra_field and left GET /field/{entity} permanently returning 500;
that array is now validated, and the registry cache is invalidated on write so
the /setting and /field paths no longer disagree until restart.
Verified against a real uvicorn server by replaying the audit's exploit: the
text/plain write is refused, a bodyless form post that would have reset a
setting is refused, a websocket from a foreign origin gets HTTP 403 on the
handshake, and clients that send no Origin at all -- Moonraker, OctoPrint,
Home Assistant, curl -- are unaffected on both HTTP and websockets.
140 lines
4.6 KiB
Python
140 lines
4.6 KiB
Python
"""Integration tests: cross-origin requests and websockets are refused.
|
|
|
|
Spoolman has no authentication, so without an origin check any website the user happens to visit
|
|
can make their browser write to a Spoolman instance it can reach -- a cross-origin form post
|
|
needs no CORS preflight -- and can open a websocket to it, since websockets are exempt from CORS
|
|
entirely.
|
|
|
|
The server under test runs with no SPOOLMAN_CORS_ORIGIN, so the policy is "same origin only".
|
|
"""
|
|
|
|
import asyncio
|
|
import json
|
|
from urllib.parse import urlsplit
|
|
|
|
import httpx
|
|
import pytest
|
|
from websockets.asyncio.client import connect
|
|
from websockets.exceptions import InvalidStatus
|
|
|
|
from .conftest import URL
|
|
|
|
WS_URL = URL.replace("http://", "ws://", 1)
|
|
|
|
OWN_HOST = urlsplit(URL).netloc
|
|
OWN_ORIGIN = URL
|
|
EVIL_ORIGIN = "https://evil.example"
|
|
|
|
|
|
def test_cross_origin_write_is_refused():
|
|
"""A write from a foreign origin must be refused, and must not change anything."""
|
|
before = httpx.get(f"{URL}/api/v1/setting/currency")
|
|
before.raise_for_status()
|
|
|
|
result = httpx.post(
|
|
f"{URL}/api/v1/setting/currency",
|
|
json='"HAX"',
|
|
headers={"Origin": EVIL_ORIGIN},
|
|
)
|
|
assert result.status_code == 403
|
|
|
|
after = httpx.get(f"{URL}/api/v1/setting/currency")
|
|
after.raise_for_status()
|
|
assert after.json() == before.json()
|
|
|
|
|
|
def test_same_origin_write_is_allowed():
|
|
"""The web UI's own origin must keep working."""
|
|
result = httpx.post(
|
|
f"{URL}/api/v1/setting/currency",
|
|
json='"SEK"',
|
|
headers={"Origin": OWN_ORIGIN},
|
|
)
|
|
result.raise_for_status()
|
|
assert result.json()["value"] == '"SEK"'
|
|
|
|
# Cleanup
|
|
httpx.post(f"{URL}/api/v1/setting/currency", json="").raise_for_status()
|
|
|
|
|
|
def test_write_without_an_origin_header_is_allowed():
|
|
"""Moonraker, OctoPrint, Home Assistant and curl send no Origin and must not be affected."""
|
|
result = httpx.post(f"{URL}/api/v1/setting/currency", json='"SEK"')
|
|
result.raise_for_status()
|
|
|
|
# Cleanup
|
|
httpx.post(f"{URL}/api/v1/setting/currency", json="").raise_for_status()
|
|
|
|
|
|
def test_cross_origin_read_is_allowed():
|
|
"""Only state-changing methods are guarded; the same-origin policy covers reads."""
|
|
result = httpx.get(f"{URL}/api/v1/spool", headers={"Origin": EVIL_ORIGIN})
|
|
assert result.status_code == 200
|
|
|
|
|
|
def test_cross_origin_delete_is_refused():
|
|
result = httpx.delete(f"{URL}/api/v1/spool/1", headers={"Origin": EVIL_ORIGIN})
|
|
assert result.status_code == 403
|
|
|
|
|
|
def test_cross_origin_backup_is_refused():
|
|
"""A bodyless form post to /backup would otherwise rotate away every restore point."""
|
|
result = httpx.post(f"{URL}/api/v1/backup", headers={"Origin": EVIL_ORIGIN})
|
|
assert result.status_code == 403
|
|
|
|
|
|
def test_forwarded_host_is_accepted():
|
|
"""A reverse proxy that rewrites Host must not have its own web UI refused."""
|
|
result = httpx.post(
|
|
f"{URL}/api/v1/setting/currency",
|
|
json='"SEK"',
|
|
headers={
|
|
"Host": "127.0.0.1:1234",
|
|
"Origin": "https://spoolman.example.com",
|
|
"X-Forwarded-Host": "spoolman.example.com",
|
|
},
|
|
)
|
|
assert result.status_code == 200
|
|
|
|
# Cleanup
|
|
httpx.post(f"{URL}/api/v1/setting/currency", json="").raise_for_status()
|
|
|
|
|
|
def test_forwarded_host_does_not_admit_a_foreign_origin():
|
|
result = httpx.post(
|
|
f"{URL}/api/v1/setting/currency",
|
|
json='"HAX"',
|
|
headers={
|
|
"Host": "127.0.0.1:1234",
|
|
"Origin": EVIL_ORIGIN,
|
|
"X-Forwarded-Host": "spoolman.example.com",
|
|
},
|
|
)
|
|
assert result.status_code == 403
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_cross_origin_websocket_is_refused():
|
|
"""Websockets bypass CORS, so this is the one channel that leaks reads cross-origin."""
|
|
with pytest.raises(InvalidStatus) as excinfo:
|
|
async with connect(f"{WS_URL}/api/v1/spool", additional_headers={"Origin": EVIL_ORIGIN}):
|
|
pass
|
|
assert excinfo.value.response.status_code == 403
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_same_origin_websocket_is_allowed():
|
|
async with connect(f"{WS_URL}/api/v1/spool", additional_headers={"Origin": OWN_ORIGIN}) as ws:
|
|
await asyncio.sleep(0.2)
|
|
await ws.send("ping")
|
|
assert json.loads(await asyncio.wait_for(ws.recv(), timeout=5))["status"] == "healthy"
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_websocket_without_an_origin_header_is_allowed():
|
|
"""Non-browser websocket consumers must keep working."""
|
|
async with connect(f"{WS_URL}/api/v1/spool") as ws:
|
|
await asyncio.sleep(0.2)
|
|
await ws.send("ping")
|
|
assert json.loads(await asyncio.wait_for(ws.recv(), timeout=5))["status"] == "healthy"
|