Skip to content

Disable XSS filter in flow step description markdown#1599

Merged
balloob merged 1 commit intohome-assistant:masterfrom
awarecan:markdown-img
Aug 26, 2018
Merged

Disable XSS filter in flow step description markdown#1599
balloob merged 1 commit intohome-assistant:masterfrom
awarecan:markdown-img

Conversation

@awarecan
Copy link
Copy Markdown
Contributor

XSS filter will filter out data:image in <image> src attribute. Disable the XSS filter allow us to use embedded image in description placeholder. Since we are fully control the step description placeholder, the XSS will not be the concern.

See: home-assistant/core#16129 (comment)

@ghost ghost assigned awarecan Aug 26, 2018
@ghost ghost added the in progress label Aug 26, 2018
@balloob balloob merged commit c8ea4cd into home-assistant:master Aug 26, 2018
@ghost ghost removed the in progress label Aug 26, 2018
@balloob
Copy link
Copy Markdown
Member

balloob commented Aug 26, 2018

Actually, I had a change of mind. It's too easy for people to update translations and this could be used as an attack vector.

Looking at the xss lib that we use, I see that we can override what attributes or tags are filtered or replaced using onTagAttr.

We should instead add an option to ha-markdown to whitelist data: in image source

@balloob
Copy link
Copy Markdown
Member

balloob commented Aug 26, 2018

Ah, found even better, we can use this:

<img src='data:image/svg+xml; ... '>

That way we don't base 64 encode. I'll just white list this type.

@balloob
Copy link
Copy Markdown
Member

balloob commented Aug 26, 2018

Was working on something but didn't finish in time. won't have time until later tonight

@awarecan
Copy link
Copy Markdown
Contributor Author

This PR has been replaced by #1600.

@awarecan awarecan deleted the markdown-img branch March 16, 2019 16:44
@github-actions github-actions bot locked and limited conversation to collaborators Jul 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants