refactor(settings-window.ts): dom xss vulnerability due to unescaped html in template literal#2023
Conversation
…html in template literal User-controlled data from `localStorage` (`key` and `panel.name`) is interpolated directly into an HTML string and assigned via `innerHTML` without sanitization. This creates a DOM-based Cross-Site Scripting (XSS) vulnerability. The `escapeHtml` utility is explicitly imported at the top of the file but mistakenly left unused. Affected files: settings-window.ts
|
@tang-vu is attempting to deploy a commit to the Elie Team on Vercel. A member of the Team first needs to authorize it. |
|
Reviewed the diff. The fix is correct and complete. The two lines changed are the only unescaped interpolations in the file that take user-sourced data. Lines 80-81 already use The attack path is narrow (requires prior localStorage poisoning) but this is a valid defense-in-depth fix consistent with the project's coding standard of escaping all output. |
Thanks for the review! Could a maintainer authorize the Vercel deployment so this can be merged? |
✨ Code Quality
Problem
User-controlled data from
localStorage(keyandpanel.name) is interpolated directly into an HTML string and assigned viainnerHTMLwithout sanitization. This creates a DOM-based Cross-Site Scripting (XSS) vulnerability. TheescapeHtmlutility is explicitly imported at the top of the file but mistakenly left unused.Severity:
highFile:
src/settings-window.tsSolution
Wrap the interpolated variables with the imported
escapeHtmlfunction:Changes
src/settings-window.ts(modified)Summary
Type of change
Affected areas
/api/*)Checklist
api/rss-proxy.jsallowlist (if adding feeds)npm run typecheck)Screenshots
🤖 About this PR
This pull request was generated by ContribAI, an AI agent
that helps improve open source projects. The change was:
If you have questions or feedback about this PR, please comment below.
We appreciate your time reviewing this contribution!
Closes #2022