Skip to content

fix(dynamic-field): escape media field value (stored XSS) - #1029

Open
mmcintosh wants to merge 1 commit into
mainfrom
fix/dynamic-field-media-value-xss
Open

fix(dynamic-field): escape media field value (stored XSS)#1029
mmcintosh wants to merge 1 commit into
mainfrom
fix/dynamic-field-media-value-xss

Conversation

@mmcintosh

Copy link
Copy Markdown
Collaborator

Description

Follow-up to #1027 (number/date/datetime value): the media field's value/url renders unescaped. The media value/url is attacker-controlled content data (stored raw by the write path) and rendered back into the content-edit form, so a content author can persist "><script>… in a media field and have it execute in an admin's edit form.

Fixes #1028

Changes

  • New local escapeJsAttr helper (\xHH) — for a single-quoted JS string inside an HTML attribute, where escapeHtml is reversible (the browser HTML-decodes the attribute before the inline onclick parses).
  • media case: value/url escaped in its HTML-attr sinks (video/img src, hidden value, data-url) via escapeHtml; the url in removeMediaFromMultiple(...) via escapeJsAttr.

Not in scope (schema-admin defense-in-depth): the field_options attrs + id/name-from-field_name sinks; the root fix there is slug-validating field_name on write.

Testing

tsc --noEmit clean. Verified in the downstream fork (Infowall) with break-it tests (media HTML sinks + onclick each go red when the escape is removed).

Unit / E2E

  • Verified downstream; can port on request

Checklist

  • Code follows project conventions
  • Type checking passes
  • No console errors or warnings
  • Documentation updated (if needed)

Companion to #1027 (number/date/datetime value): the media value/url is attacker-controlled
content data (stored raw by the write path) and rendered back into the content-edit form.

- New local escapeJsAttr helper (\xHH) — for a single-quoted JS string inside an HTML attribute,
  where escapeHtml is reversible (the browser HTML-decodes the attribute before the inline
  onclick parses).
- media case: media value/url escaped in its HTML-attr sinks (video/img src, hidden value,
  data-url) via escapeHtml; the url in removeMediaFromMultiple(...) via escapeJsAttr.

Not in scope (schema-admin defense-in-depth): the field_options attrs + id/name-from-field_name
sinks; the root fix there is slug-validating field_name on write.
@mmcintosh
mmcintosh marked this pull request as ready for review July 24, 2026 01:08
@mmcintosh
mmcintosh requested a review from lane711 as a code owner July 24, 2026 01:08

@lane711 lane711 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Fix is correct. escapeJsAttr logic sound — \x27/\x22 avoids the HTML-decode-before-parse pitfall. One pre-existing gap scoped out correctly (tracked separately): fieldId unescaped in onclick — root fix is slug-validating field_name on write.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security: media field value renders unescaped (stored XSS)

2 participants