Skip to content

[Codex] fix(security): sanitize SVG uploads and restrict icon import to admins#555

Open
Micsi wants to merge 4 commits into
abeggled:mainfrom
Micsi:codex/propose-fix-for-svg-xss-vulnerability
Open

[Codex] fix(security): sanitize SVG uploads and restrict icon import to admins#555
Micsi wants to merge 4 commits into
abeggled:mainfrom
Micsi:codex/propose-fix-for-svg-xss-vulnerability

Conversation

@Micsi
Copy link
Copy Markdown
Collaborator

@Micsi Micsi commented May 26, 2026

Motivation

  • Close a stored-XSS vector where arbitrary SVG files uploaded by any authenticated user were stored verbatim and later injected into the Settings UI via v-html.

Description

  • Add a _sanitize_svg(content: bytes) -> bytes helper that parses SVG XML and removes risky constructs and attributes (strips script/foreignObject/iframe/object/embed, removes on* event attributes and javascript: hrefs) using xml.etree.ElementTree and raises HTTPException(422) for invalid XML.
  • Persist sanitized SVG bytes instead of raw uploads when writing single-file uploads and ZIP members by calling _sanitize_svg(...) before write_bytes.
  • Require admin privileges for the import endpoint by switching the dependency from get_current_user to get_admin_user on POST /api/v1/icons/import to prevent low-privileged users from planting payloads.
  • Add unit tests in tests/unit/test_icons.py for the sanitizer behavior (event-handler stripping, removal of script/foreignObject, and rejection of invalid XML).

Testing

  • Ran pytest -q tests/unit/test_icons.py and all unit tests passed (27 passed, 1 warning).
  • Attempted to run the integration suite (tests/integration/test_icons.py) but the environment lacked pytest_asyncio, so the integration tests were not executed here (local CI should run the full integration suite).

Codex Task

@Micsi
Copy link
Copy Markdown
Collaborator Author

Micsi commented May 26, 2026

🔄 Promotion-Update aus dem Fork

Diese Upstream-PR wurde in dieser Session mit dem aktuellen Stand aus Micsi#48 synchronisiert.

Synchronisierte Änderungen

  • API-Key-Owner wird bei Username-Rename mitgezogen (api_keys.owner), damit Admin/API-Key-Auth nach Umbenennungen stabil bleibt.
  • ZIP-Import für Icons ist jetzt atomar: bei Sanitizer-Fehlern gibt es keine Teil-Schreibvorgänge mehr.
  • Ergänzende Integrations-Tests für beide Fälle sowie Update der RELEASENOTES.md.

Review-Setup

Enthaltener Commit-Stand (gegen origin/main)

  • 9a6425e fix(security): sanitize uploaded SVG icons and restrict imports to admins
  • 878a23a fix(security): harden svg sanitizer and keep admin api-key imports
  • bec3137 fix(security): sync api-key owner on rename and make zip import atomic

Lokale Verifikation inkl. Pre-Push-Gates war grün (i18n guard, ruff check, ruff format --check, pytest).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Security Security-related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant