fix: strip event handlers in htmlToFragment before compose DOM insertion#171
Open
JuliaKalder wants to merge 1 commit into
Open
fix: strip event handlers in htmlToFragment before compose DOM insertion#171JuliaKalder wants to merge 1 commit into
JuliaKalder wants to merge 1 commit into
Conversation
Adds stripDangerousContent() that walks the imported node tree and removes all on* attributes, plus javascript: and data: URIs from src/href. Called in htmlToFragment() for every imported element node before it is appended to the fragment that Range.insertNode places into the compose-window DOM. The compose window is not governed by the extension's script-src 'self' CSP, so inline event handlers would execute immediately on insertion. Template HTML can originate from saved emails or imported JSON files, both of which are attacker-controlled sources. Fixes #161
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Strips inline event-handler attributes (
on*) and dangerous URI schemes from DOM nodes before inserting template HTML into the Thunderbird compose window in cursor mode.Problem
Template HTML was inserted directly into the Thunderbird compose editor DOM via
Range.insertNodewithout stripping inline event handlers. The compose window is not governed by the extension'sscript-src 'self'CSP, soon*attributes (onerror,onload,onmouseover, etc.) fire immediately upon DOM insertion.Template HTML can reach this code from attacker-controlled sources:
background.js:158–163stores raw email HTML verbatim as a templateChanges
stripDangerousContent(root)usingTreeWalkerto remove allon*attributes from every element in the imported node treejavascript:anddata:URIs fromsrc/hrefattributes to block navigation-based XSShtmlToFragment()to callstripDangerousContent()on each imported element node before appending to the fragmentTesting
All 96 existing tests pass. The fix is minimal and isolated to
modules/compose-script.js.Fixes #161