Skip to content

[Codex] fix(security): sanitize SVG icons before ValueDisplay injection#556

Merged
abeggled merged 2 commits into
abeggled:mainfrom
Micsi:codex/fix-stored-xss-in-valuedisplay-svg-handling
May 27, 2026
Merged

[Codex] fix(security): sanitize SVG icons before ValueDisplay injection#556
abeggled merged 2 commits into
abeggled:mainfrom
Micsi:codex/fix-stored-xss-in-valuedisplay-svg-handling

Conversation

@Micsi

@Micsi Micsi commented May 26, 2026

Copy link
Copy Markdown
Collaborator

Motivation

  • The ValueDisplay widget began rendering imported SVG icon markup with v-html, creating a stored XSS sink when the icons API returns raw uploaded SVG.
  • The intent is to eliminate the XSS vector while preserving the existing widget behavior and icon selection UI by sanitizing icons before they reach v-html.

Description

  • Replaced the previous normalizeSvg approach with a sanitizeSvg implementation in frontend/src/composables/useIcons.ts that parses the SVG via DOMParser and returns a cleaned root SVG string.
  • The sanitizer removes executable/HTML-capable elements (script, foreignObject), strips inline event-handler attributes (on*), and removes javascript: payloads from href/xlink:href attributes.
  • The sanitizer also removes fixed width/height attributes to preserve the original CSS-controlled sizing behavior, and the icon cache now stores sanitized output via svgCache[icon.name] = sanitizeSvg(icon.content).
  • No changes were made to widget templates or the icons upload/list API in this patch; sanitization is applied client-side at the composable layer before any v-html usage.

Testing

  • Ran type checking with npm --prefix frontend run typecheck (which runs vue-tsc --noEmit) and it completed successfully.

Codex Task

@Micsi
Micsi requested a review from abeggled May 26, 2026 07:08
@Micsi Micsi added the Security Security-related changes label May 26, 2026
@Micsi Micsi self-assigned this May 26, 2026

@abeggled abeggled left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Danke

@abeggled
abeggled merged commit cc66287 into abeggled:main May 27, 2026
6 checks passed
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.

2 participants