Skip to content

Build the library browser alert toast with DOM methods to avoid DOM XSS#3054

Open
xcompass wants to merge 1 commit into
openwebwork:WeBWorK-2.21from
ubc:upstream-setmaker-toast-xss
Open

Build the library browser alert toast with DOM methods to avoid DOM XSS#3054
xcompass wants to merge 1 commit into
openwebwork:WeBWorK-2.21from
ubc:upstream-setmaker-toast-xss

Conversation

@xcompass

@xcompass xcompass commented Jul 13, 2026

Copy link
Copy Markdown
Member

Summary

The alertToast helper in htdocs/js/SetMaker/setmaker.js (the library browser) interpolated its title and msg arguments directly into an innerHTML template. All current callers pass plain text, but several of those strings incorporate server-provided content (e.g. RPC error messages), and interpolating DOM text into innerHTML lets it be reinterpreted as HTML. CodeQL flags this pattern as a js/xss-through-dom alert.

Per review, this now removes the innerHTML usage entirely: the toast is built with document.createElement and the title and message are assigned with textContent, so caller-supplied strings are always treated as text and can never be parsed as markup. The construction mirrors the toast already built this way in htdocs/js/TagWidget/tagwidget.js.

The separate alertToast in htdocs/js/GatewayQuiz/gateway.js is intentionally left alone — it renders HTML from server-controlled data attributes by design.

Testing

  • node --check htdocs/js/SetMaker/setmaker.js passes (Node 20).
  • prettier --check on the modified file passes (matches the check-formats CI workflow).
  • Behavior is unchanged for all existing callers: the rendered toast markup is identical (same elements, classes, and attributes as the previous template); a title or message containing markup characters displays literally instead of being parsed as HTML.

@drgrice1 drgrice1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I was a bit lazy when I implemented this and used innterHTML. It would be better to remove that entirely and build the HTML with document.createElement rather than using innterHTML and then querySelector to find the contents.

The alertToast helper in the library browser interpolated its title and
msg arguments directly into an innerHTML template, letting DOM text be
reinterpreted as HTML (CodeQL js/xss-through-dom). Build the toast with
document.createElement instead and assign the title and message with
textContent, so caller-supplied strings are always treated as text. This
matches the toast construction already used in htdocs/js/TagWidget.
@xcompass xcompass force-pushed the upstream-setmaker-toast-xss branch from 309af66 to 9b7c626 Compare July 13, 2026 22:42
@xcompass xcompass changed the title Set library browser alert toast title/message via textContent to avoid DOM XSS Build the library browser alert toast with DOM methods to avoid DOM XSS Jul 13, 2026
@xcompass

Copy link
Copy Markdown
Member Author

Done — removed the innerHTML usage entirely; the toast is now built with document.createElement and textContent, following the same construction as the toast in htdocs/js/TagWidget/tagwidget.js.

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.

2 participants