Skip to content

Commit 80388b1

Browse files
lsteinclaude
andauthored
fix(security): close three HIGH-severity API issues (#224)
* fix(security): close three HIGH-severity API issues * Thumbnail ``color`` / ``size`` / ``radius`` are now whitelisted before the ``color`` string reaches the cache-filename construction. Values like ``?color=../../evil`` previously escaped the thumbnail cache directory via ``Path /`` concatenation and let PIL write a .png to an arbitrary location. * ``POST /version/update`` and ``POST /version/restart`` now honour ``PHOTOMAP_INLINE_UPGRADE`` server-side (previously only the UI button was hidden) and require an ``X-Requested-With: photomap`` header, which forces a CORS preflight that we do not answer — so a cross-origin simple POST from any page the user visits can no longer silently trigger a pip install or kill the server. The frontend already sends the header from ``about.js``. * InvokeAI settings reject URLs whose scheme is not ``http``/``https`` (and empty-host values like ``http://``) so the config field cannot be flipped to ``file://`` or ``javascript:`` and used as an SSRF pivot from ``/status``, ``/boards``, ``/recall`` or ``/use_ref_image``. The ``queue_id`` field is constrained to ``[A-Za-z0-9_.-]{1,64}`` so a request body can't splice ``../`` into the outbound path and reach arbitrary endpoints on the configured backend. New tests: tests/backend/test_thumbnail_validation.py, tests/backend/test_upgrade_router.py, plus SSRF / queue-id coverage appended to tests/backend/test_invoke_router.py. 175 backend + 239 frontend tests pass; ruff / eslint / prettier clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(security): close add_album → serve_image arbitrary-file-read chain ``POST /add_album`` accepts arbitrary absolute ``image_paths``, and ``serve_image`` / ``get_image_by_name`` previously returned any file under those paths as long as ``validate_image_access``'s ``is_relative_to`` guard passed — the guard checks *location* only, not *type*. A caller could therefore create an album with ``image_paths=["/etc"]`` and read ``/etc/passwd`` via ``GET /images/<key>/passwd``. Both endpoints now require the resolved file's suffix to be in ``SUPPORTED_EXTENSIONS`` (the same allowlist the indexer uses). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(backend): PHOTOMAP_INLINE_UPGRADE env variable now works as documented * fix(settings): surface InvokeAI URL validation and reachability errors inline Previously the settings panel silently dropped the 400 returned for invalid URLs (e.g. file:// schemes) and only flipped auth-row visibility for unreachable backends, leaving the user with no feedback. The hint under the URL field now turns red with a warning icon and shows the backend's detail for: invalid scheme/host, unreachable host, and reachable-but-not-InvokeAI servers. Restores the default hint once the URL is valid and reachable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 61bc26e commit 80388b1

11 files changed

Lines changed: 510 additions & 23 deletions

File tree

photomap/backend/args.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,6 @@ def get_args() -> argparse.Namespace:
7272
"--inline-upgrade",
7373
action=argparse.BooleanOptionalAction,
7474
default=True,
75-
help="Perform inline database upgrades",
75+
help="Allow inline package upgrades from the About dialog (default: True). Overridden by PHOTOMAP_INLINE_UPGRADE env var.",
7676
)
7777
return parser.parse_args()

photomap/backend/photomap_server.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ def main():
202202
# Convert list of album keys to comma-separated string
203203
os.environ["PHOTOMAP_ALBUM_LOCKED"] = ",".join(args.album_locked)
204204

205-
os.environ["PHOTOMAP_INLINE_UPGRADE"] = "1" if args.inline_upgrade else "0"
205+
os.environ.setdefault("PHOTOMAP_INLINE_UPGRADE", "1" if args.inline_upgrade else "0")
206206

207207
app_url = get_app_url(host, port)
208208

photomap/backend/routers/invoke.py

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import time
3131
from collections.abc import Awaitable, Callable
3232
from pathlib import Path
33+
from urllib.parse import urlsplit
3334

3435
import httpx
3536
from fastapi import APIRouter, HTTPException
@@ -212,6 +213,44 @@ async def _do(headers: dict[str, str]) -> httpx.Response:
212213
return resp.status_code == 200
213214

214215

216+
def _validate_invokeai_url(url: str | None) -> str | None:
217+
"""Reject non-http(s) schemes so configured URLs cannot be used for SSRF.
218+
219+
The configured URL is later concatenated into outbound requests for
220+
``/status``, ``/boards``, ``/recall`` and ``/use_ref_image``; ``httpx``
221+
already refuses non-http(s) schemes, but validating up front returns
222+
a clean 400 to the settings panel rather than a 502 at call time, and
223+
blocks obviously-wrong values like ``file://`` or ``javascript:`` from
224+
ever reaching the config file.
225+
226+
Empty / None is allowed — that's "not configured yet".
227+
"""
228+
if not url:
229+
return url
230+
try:
231+
parts = urlsplit(url)
232+
except ValueError as exc:
233+
raise HTTPException(
234+
status_code=400, detail=f"Invalid InvokeAI URL: {exc}"
235+
) from exc
236+
if parts.scheme not in {"http", "https"}:
237+
raise HTTPException(
238+
status_code=400,
239+
detail="InvokeAI URL must use http:// or https://",
240+
)
241+
if not parts.netloc:
242+
raise HTTPException(
243+
status_code=400, detail="InvokeAI URL must include a host"
244+
)
245+
return url
246+
247+
248+
# InvokeAI queue ids are short opaque tokens (e.g. ``default``); restrict
249+
# the pattern so a caller can't splice ``../`` into the outbound URL path
250+
# and reach an arbitrary endpoint on the configured backend.
251+
_QUEUE_ID_PATTERN = r"^[A-Za-z0-9_.-]{1,64}$"
252+
253+
215254
class InvokeAISettings(BaseModel):
216255
"""Mirrors the config fields we expose via the settings panel."""
217256

@@ -230,15 +269,23 @@ class RecallRequest(BaseModel):
230269
True,
231270
description="If False, omit the seed from the recall payload (remix mode)",
232271
)
233-
queue_id: str = Field("default", description="InvokeAI queue id to target")
272+
queue_id: str = Field(
273+
"default",
274+
description="InvokeAI queue id to target",
275+
pattern=_QUEUE_ID_PATTERN,
276+
)
234277

235278

236279
class UseRefImageRequest(BaseModel):
237280
"""Payload posted by the drawer's "Use Ref Image" button."""
238281

239282
album_key: str = Field(..., description="Album containing the image")
240283
index: int = Field(..., ge=0, description="Image index within the album")
241-
queue_id: str = Field("default", description="InvokeAI queue id to target")
284+
queue_id: str = Field(
285+
"default",
286+
description="InvokeAI queue id to target",
287+
pattern=_QUEUE_ID_PATTERN,
288+
)
242289

243290

244291
@invoke_router.get("/config")
@@ -265,13 +312,14 @@ async def set_invokeai_config(settings: InvokeAISettings) -> dict:
265312
untouched so the settings panel can PATCH individual fields without
266313
clobbering what was saved. Send an empty string to explicitly clear.
267314
"""
315+
url = _validate_invokeai_url(settings.url)
268316
_invalidate_token_cache()
269317
existing = config_manager.get_invokeai_settings()
270318
password = settings.password if settings.password is not None else existing["password"]
271319
board_id = settings.board_id if settings.board_id is not None else existing["board_id"]
272320
try:
273321
config_manager.set_invokeai_settings(
274-
url=settings.url,
322+
url=url,
275323
username=settings.username,
276324
password=password,
277325
board_id=board_id,
@@ -312,15 +360,16 @@ async def invokeai_status() -> dict:
312360
"reachable": False,
313361
"detail": f"Backend returned HTTP {resp.status_code}",
314362
}
363+
not_invokeai = "Server is reachable but doesn't appear to be an InvokeAI backend"
315364
try:
316365
payload = resp.json()
317366
except ValueError:
318-
return {"reachable": False, "detail": "Backend did not return JSON"}
367+
return {"reachable": False, "detail": not_invokeai}
319368
version = payload.get("version")
320369
if not version:
321370
# A non-InvokeAI server happening to have /api/v1/app/version would
322371
# almost certainly not return a version field.
323-
return {"reachable": False, "detail": "Response missing version field"}
372+
return {"reachable": False, "detail": not_invokeai}
324373
return {"reachable": True, "version": version}
325374

326375

photomap/backend/routers/search.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import base64
99
import json
10+
import re
1011
import zipfile
1112
from io import BytesIO
1213
from logging import getLogger
@@ -18,6 +19,7 @@
1819
from pydantic import BaseModel
1920

2021
from ..config import get_config_manager
22+
from ..embeddings import SUPPORTED_EXTENSIONS
2123
from ..metadata_modules import SlideSummary
2224
from .album import (
2325
get_embeddings_for_album,
@@ -29,6 +31,15 @@
2931
search_router = APIRouter()
3032
logger = getLogger(__name__)
3133

34+
# The ``color`` query param is interpolated into the on-disk thumbnail
35+
# cache filename, so anything that survives here becomes a path segment.
36+
# Accept only a 6-digit hex literal (with or without ``#``) or an
37+
# ``r,g,b`` CSV of three 0-255 integers — reject everything else so a
38+
# value like ``../../evil`` cannot escape the thumbnail cache dir.
39+
_COLOR_RE = re.compile(r"\A#?[0-9A-Fa-f]{6}\Z|\A\d{1,3},\d{1,3},\d{1,3}\Z")
40+
_MAX_THUMB_SIZE = 2048
41+
_MAX_THUMB_RADIUS = 512
42+
3243

3344
# Response Models
3445
class SearchResult(BaseModel):
@@ -181,6 +192,13 @@ async def serve_thumbnail(
181192
radius: int = 12, # Add a radius parameter for rounded corners
182193
) -> FileResponse:
183194
"""Serve a reduced-size thumbnail for an image by index, with optional colored border."""
195+
if size <= 0 or size > _MAX_THUMB_SIZE:
196+
raise HTTPException(status_code=400, detail="Invalid thumbnail size")
197+
if radius < 0 or radius > _MAX_THUMB_RADIUS:
198+
raise HTTPException(status_code=400, detail="Invalid thumbnail radius")
199+
if color is not None and not _COLOR_RE.match(color):
200+
raise HTTPException(status_code=400, detail="Invalid color parameter")
201+
184202
embeddings = get_embeddings_for_album(album_key)
185203
try:
186204
image_path = embeddings.get_image_path(index)
@@ -266,6 +284,14 @@ async def serve_image(album_key: str, path: str):
266284
if not validate_image_access(album_config, image_path):
267285
raise HTTPException(status_code=403, detail="Access denied")
268286

287+
# Enforce the image-extension allowlist on any file-serving endpoint.
288+
# ``add_album`` accepts arbitrary absolute ``image_paths``; without this
289+
# check a caller could point an album at ``/etc`` and then read
290+
# ``/images/<key>/passwd`` (the ``is_relative_to`` guard above only
291+
# checks *location*, not type).
292+
if image_path.suffix.lower() not in SUPPORTED_EXTENSIONS:
293+
raise HTTPException(status_code=403, detail="Unsupported image type")
294+
269295
if not image_path.exists() or not image_path.is_file():
270296
raise HTTPException(status_code=404, detail="File not found")
271297

@@ -345,6 +371,9 @@ async def get_image_by_name(album_key: str, filename: str) -> FileResponse:
345371
"""
346372
Serve an image by its filename within the specified album.
347373
"""
374+
if Path(filename).suffix.lower() not in SUPPORTED_EXTENSIONS:
375+
raise HTTPException(status_code=403, detail="Unsupported image type")
376+
348377
embeddings = get_embeddings_for_album(album_key)
349378
if not embeddings:
350379
raise HTTPException(status_code=404, detail="Album not found")

photomap/backend/routers/upgrade.py

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,46 @@
88
from logging import getLogger
99

1010
import requests
11-
from fastapi import APIRouter
11+
from fastapi import APIRouter, HTTPException, Request
1212
from fastapi.responses import JSONResponse
1313
from packaging import version as pversion
1414

1515
upgrade_router = APIRouter()
1616
logger = getLogger(__name__)
1717

1818

19+
def _require_inline_upgrades_enabled() -> None:
20+
"""Honour the ``PHOTOMAP_INLINE_UPGRADE`` deployment switch.
21+
22+
The flag is set from ``--inline-upgrade`` / env on startup; when the
23+
operator has explicitly disabled it the UI hides the button, but the
24+
endpoint was previously still callable. Enforce it here so the backend
25+
is the source of truth.
26+
"""
27+
if os.environ.get("PHOTOMAP_INLINE_UPGRADE", "1") != "1":
28+
raise HTTPException(
29+
status_code=403,
30+
detail="Inline upgrades are disabled on this deployment.",
31+
)
32+
33+
34+
def _require_same_origin_header(request: Request) -> None:
35+
"""Reject requests that lack the ``X-Requested-With`` marker.
36+
37+
The update and restart endpoints perform side effects (pip install,
38+
process kill) and have no authentication. A cross-origin page could
39+
otherwise submit a CSRF-able simple POST to ``http://localhost:8050``
40+
and trigger either action. Requiring a non-standard request header
41+
forces the caller through a CORS preflight, which our server does not
42+
answer — so only same-origin JS with an explicit header succeeds.
43+
"""
44+
if request.headers.get("x-requested-with") != "photomap":
45+
raise HTTPException(
46+
status_code=403,
47+
detail="Missing required X-Requested-With header.",
48+
)
49+
50+
1951
@upgrade_router.get("/version/check", tags=["Upgrade"])
2052
async def check_version():
2153
"""Check if a newer version is available on PyPI"""
@@ -55,8 +87,10 @@ async def check_version():
5587

5688

5789
@upgrade_router.post("/version/update", tags=["Upgrade"])
58-
async def update_version():
90+
async def update_version(request: Request):
5991
"""Update PhotoMapAI to the latest version using pip"""
92+
_require_inline_upgrades_enabled()
93+
_require_same_origin_header(request)
6094
try:
6195
# Run pip install --upgrade photomapai
6296
result = subprocess.run(
@@ -98,8 +132,10 @@ async def update_version():
98132

99133

100134
@upgrade_router.post("/version/restart", tags=["Upgrade"])
101-
async def restart_server():
135+
async def restart_server(request: Request):
102136
"""Restart the server after update"""
137+
_require_inline_upgrades_enabled()
138+
_require_same_origin_header(request)
103139

104140
def delayed_restart():
105141
time.sleep(2) # Give time for response to be sent

photomap/frontend/static/javascript/about.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ class AboutManager {
116116
try {
117117
const response = await fetch("version/update", {
118118
method: "POST",
119+
headers: { "X-Requested-With": "photomap" },
119120
});
120121
const data = await response.json();
121122

@@ -128,7 +129,10 @@ class AboutManager {
128129
if (data.restart_available) {
129130
setTimeout(async () => {
130131
try {
131-
await fetch("version/restart", { method: "POST" });
132+
await fetch("version/restart", {
133+
method: "POST",
134+
headers: { "X-Requested-With": "photomap" },
135+
});
132136
updateStatus.textContent = "Server restarting... Waiting for server to come back online...";
133137

134138
// Wait 5 seconds before starting to poll

0 commit comments

Comments
 (0)