Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not allow SVG by default #3640

Merged
merged 1 commit into from Sep 7, 2019

Conversation

@balloob
Copy link
Member

commented Sep 7, 2019

Today I got a report by Gregor Godbersen that our markdown XSS filter was ineffective because we always allowed SVG tags, which can be exploited with the following markdown:

<svg onload='alert(1234)'></svg>

Filtering SVG tags by default was removed in #3458 because no one could remember why it was there. Thanks to Gregor we remember.

This issue only impacts Home Assistant 0.98. The <ha-markdown> tag is used in both Home Assistant and Hass.io. In Home Assistant it is used to render config flows, auth flows, config option flows. These all render trusted content: either translations or content the user set in their markdown card.

Hass.io has not had a UI release since #3458 was merged, so it was not impacted. Hass.io does render external markdown (add-on descriptions) but the existing XSS was in place.

@balloob balloob merged commit 2ff4d0f into dev Sep 7, 2019

3 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/push The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details

@delete-merged-branch delete-merged-branch bot deleted the markdown-whitelist-svg branch Sep 7, 2019

balloob added a commit that referenced this pull request Sep 7, 2019
@balloob balloob referenced this pull request Sep 7, 2019
@bramkragten bramkragten referenced this pull request Sep 8, 2019
@awarecan

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2019

allowsvg was introduced to display QR code in mfa module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.