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

Allow SVG elements in markdown #1087

Merged
merged 2 commits into from
May 5, 2020
Merged

Allow SVG elements in markdown #1087

merged 2 commits into from
May 5, 2020

Conversation

jameshadfield
Copy link
Member

@jameshadfield jameshadfield commented Apr 24, 2020

This modifies our settings for DOMpurify to allow SVG tags (and tags used within SVG).

You can see a test narrative here. This will be available vie the review app also.

For this to work we removed the ALLOWED_ATTR sanitizer config settings, but I note that the docs state "The included default configuration values are pretty good already". An alternate approach would be to also remove the ALLOWED_TAGS setting as the default allows SVG.

@jameshadfield jameshadfield temporarily deployed to auspice-svg-in-md-7zf9dhardt4m April 24, 2020 09:28 Inactive
@joverlee521
Copy link
Contributor

I'm definitely not an expert on script injection and not sure what "default configuration values are pretty good already" means.

When I made these initial changes to the footer, @tsibley had commented that allowing style will allow clickjacking. Perhaps we want to add {FORBID_ATTR: ['style']}?

@tsibley
Copy link
Member

tsibley commented Apr 27, 2020

I don't think it's wise to remove our ALLOWED_ATTR or switch to only FORBID_ATTR. DOMPurify's goals are very focused on XSS and not other issues arising from embedding untrusted content and its defaults for allowed content reflect this.

@jameshadfield
Copy link
Member Author

I don't think it's wise to remove our ALLOWED_ATTR or switch to only FORBID_ATTR

I have no problem keeping ALLOWED_ATTR if it's possible to find a list of values which allow SVG rendering.

Slight changes in function in that table elements are now allowed in the footer as well as the main-narrative-markdown. This was commented as a "to-do" in the code
This allows us to embed SVG images in markdown
@jameshadfield jameshadfield temporarily deployed to auspice-svg-in-md-7zf9dhardt4m April 28, 2020 01:19 Inactive
@jameshadfield jameshadfield merged commit 37aece4 into master May 5, 2020
@jameshadfield jameshadfield deleted the svg-in-md branch May 5, 2020 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants