Skip to content

RS-22478: fix stored XSS in metro HTML labels#51

Merged
chschan merged 4 commits into
masterfrom
RS-22478-fix-label-xss
Jun 15, 2026
Merged

RS-22478: fix stored XSS in metro HTML labels#51
chschan merged 4 commits into
masterfrom
RS-22478-fix-label-xss

Conversation

@chschan

@chschan chschan commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Sanitise as_html label content with DOMPurify before rendering it, keeping the render-as-HTML feature while stripping scripts/handlers. Adds the dompurify dependency.

chschan and others added 2 commits June 12, 2026 15:39
Sanitise as_html label content with DOMPurify before rendering it, keeping
the render-as-HTML feature while stripping scripts/handlers. Adds the
dompurify dependency. Rebuilt the inst/htmlwidgets bundle.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@chschan chschan requested a review from JustinCCYap June 12, 2026 05:46
@JustinCCYap

Copy link
Copy Markdown
Contributor

This is perhaps risker than the other changes as it has bigger potential to break existing outputs. The standard R tests should provide some assurance. Check them before promoting the R server.

@JustinCCYap JustinCCYap left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@chschan

chschan commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

You're right this causes a lot of diffs. Also by default rhtmlMetro is run in an iframe (which is already safe). Will look into checks around calls to boxIframeless in flipFormat where we create custom widgets for Standard R outputs.

@chschan chschan closed this Jun 15, 2026
@chschan chschan reopened this Jun 15, 2026
@chschan

chschan commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

On further inspection, the iframe gives functional isolation (a widget's CSS/JS/global vars don't clobber the rest of Displayr) but no XSS isolation. Its same-origin and non-sandboxed. So we'll try this again with sanitization (instead of removal of html elements).

@JustinCCYap

Copy link
Copy Markdown
Contributor

On further inspection, the iframe gives functional isolation (a widget's CSS/JS/global vars don't clobber the rest of Displayr) but no XSS isolation. Its same-origin and non-sandboxed. So we'll try this again with sanitization (instead of removal of html elements).

OK

@chschan chschan merged commit f95425c into master Jun 15, 2026
2 checks passed
JustinCCYap added a commit that referenced this pull request Jun 16, 2026
JustinCCYap added a commit that referenced this pull request Jun 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants