Skip to content

[Codex] security: reject active/scriptable SVGs on icon import (prevent stored XSS)#558

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

[Codex] security: reject active/scriptable SVGs on icon import (prevent stored XSS)#558
Micsi wants to merge 2 commits into
abeggled:mainfrom
Micsi:codex/propose-fix-for-unsanitized-svg-xss

Conversation

@Micsi
Copy link
Copy Markdown
Collaborator

@Micsi Micsi commented May 26, 2026

Motivation

  • The config import and icon upload paths accepted attacker-controlled SVG bytes after a shallow <svg> check, and stored SVGs were later returned verbatim and rendered with v-html, enabling stored XSS.
  • The change aims to block scriptable/active SVG constructs at ingestion so unsafe SVG content cannot be written to disk or later served to clients.

Description

  • Added a server-side sanitizer _sanitize_svg in obs/api/v1/icons.py that rejects SVGs containing active/scriptable constructs such as <script>, <foreignObject>, inline on*= handlers, and javascript:/data: links.
  • Applied the sanitizer to the dedicated icon upload endpoint (/api/v1/icons/import) for both single-file and ZIP-member flows so only safe SVG bytes are written.
  • Applied the same sanitizer to the config restore flow in obs/api/v1/config.py so base64-decoded icons from config imports are validated before being stored.
  • Added unit tests in tests/unit/test_icons.py that assert acceptance of minimal SVG and rejection of malicious patterns.

Testing

  • Ran pytest -q tests/unit/test_icons.py and all unit tests passed (28 passed).
  • Attempted to run the full integration suite, but integration tests could not run in this environment due to a missing pytest_asyncio dependency (so integration tests were not executed here).

Codex Task

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