fix: Sanitize HTML tooltips #2538
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
github-actions
bot
added
the
front-end
|
Uffizzi Preview |
frontend/web/components/Toolip.js
Outdated
| @@ -16,7 +16,7 @@ const Tooltip = class extends React.Component { | |||
| <span className='ion ion-ios-help' data-for={this.id} data-tip /> | |||
| )} | |||
| <ReactTooltip | |||
| html | |||
| html={false} | |||
There was a problem hiding this comment.
We only want to disable html for some usages, a lot of the times we use tooltips e.g. permissions uses html for tooltip content.
There was a problem hiding this comment.
Implementing DOMPurify to sanitize the tooltip content since it removes XSS scripts but not HTML content.
There was a problem hiding this comment.
As per our conversation, proceeded to HTML encode the tag labels to escape HTML and display content as plain text.
Also, converted Tooltip code to Typescript functional component.
frontend/web/components/tags/Tag.tsx
Outdated
| @@ -47,6 +47,7 @@ const Tag: FC<TagType> = ({ | |||
|
|
|||
| return ( | |||
| <Tooltip | |||
| htmlEncode | |||
There was a problem hiding this comment.
we'd probably want to make the property naming of this a bit more clear.
I think plainText might be good since in this case you're wanting to make the tooltip content text rather than html.
There was a problem hiding this comment.
Sure, that was my first option but then I thought it was not descriptive enough :). Done
There was a problem hiding this comment.
BTW, also fixed the component filename from toolip to tooltip.
frontend/web/components/Tooltip.tsx
Outdated
| children: string | JSX.Element | JSX.Element[] | (() => JSX.Element), | ||
| ): string => { | ||
| const html = renderToStaticMarkup( | ||
| <StyledTooltip>{plainText ? children : '{{placeholder}}'}</StyledTooltip>, |
There was a problem hiding this comment.
Is {{placeholder}} supposed to be here?
There was a problem hiding this comment.
Replace with {{html}} for clarity, also, some issues with typescript were corrected
Thanks for submitting a PR! Please check the boxes below:
pre-committo check lintingChanges
DIsable HTML tooltips since they could cause XSS injection vulnerability
How did you test this code?
Manually.
" as the label of a tag.
Insert XSS code like "xss
When hover over the tag, it should display simple text and not HTML.