Conversation
emersion
force-pushed
the
note-xss
branch
2 times, most recently
from
6594b05 to
fa4fabb
Compare
This was referenced Dec 19, 2025
Notes can contain HTML in their body text. The HTML was directly piped into the DOM in 3 spots, resulting in an XSS vulnerability. The XSS vulnerability can be used to e.g. perform authenticated API calls on behalf of another user by making a privileged user open a shared variant by sending them a link. Here are the 3 spots where the note text is used: - In the notes view, when displaying the body text in the editor main view's SVG with d3. The d3 html() method is used. - In the notes view, when computing the size of the body text. The innerHTML field is set. - In the note edit dialog, when passing the text to ngx-editor. ngx-editor doesn't perform any sanitization on its own. Fix all of these by adding DomSanitizer.sanitize() calls. Signed-off-by: Simon Ser <contact@emersion.fr>
aiAdrian
requested changes
Jan 5, 2026
Contributor
aiAdrian
left a comment
aiAdrian
left a comment
There was a problem hiding this comment.
The proposed change will no longer provide the same behaviour... Colours are no longer be visible in the note dialog.
Before change
After change (this PR)
Contributor
chrome-capture-2026-01-05.webm |
Member
Author
|
Indeed, good catch! Hm, this is going to be tricker to fix… It doesn't seem like there is a way to convince |
louisgreiner
added
area:view
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
3 participants
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.
Notes can contain HTML in their body text. The HTML was directly piped into the DOM in 3 spots, resulting in an XSS vulnerability. The XSS vulnerability can be used to e.g. perform authenticated API calls on behalf of another user by making a privileged user open a shared variant by sending them a link.
Here are the 3 spots where the note text is used:
Fix all of these by adding DomSanitizer.sanitize() calls.
Here is a PoC for the XSS vulnerability:
networkGraphic(16).json