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

Don't use object tag (for SVGs) #6162

Closed
rugk opened this issue Feb 15, 2018 · 6 comments · Fixed by #17818
Closed

Don't use object tag (for SVGs) #6162

rugk opened this issue Feb 15, 2018 · 6 comments · Fixed by #17818

Comments

@rugk
Copy link
Contributor

rugk commented Feb 15, 2018

In order to make it good for a CSP (#3632) a big thing is just to remove that object tag. It is usually used to embed flash or other "active content", which nobody uses anymore, so in order to block flash and other nasty things, you have not use the object tag

I also don't see any reason. SVGs are images, so use svg.

I read this and maybe you use it to manipulate them from teh outside? In this case, BTW, you can always just embed the SVG itself in the HTML, which is a nicer solution than using object.

@ara4n
Copy link
Member

ara4n commented Feb 17, 2018

the reason we use <object/> is to allow the SVG CSS to be manipulated from the outside in order to tint. I'm unclear why embedding SVG directly in the DOM any better?

@rugk
Copy link
Contributor Author

rugk commented Feb 17, 2018

It would be good for #3632, i.e. for creating a strong CSP. Images (img tag) are covered by img-src there, and of course allowed for self.
To allow the object tag, we have to allow them using object-src. object-src, however, includes many things, which are not used by Riot, including flash plugins and stuff like that. Google's CSP validator also always complains if you specify a CSP and set no value for object-src, pointing out that this should be disallowed and may also be used to bypass the CSP, because of course flash or Java or whatever plugins, do not abide the CSP. (don't know the details here, how they act with same origin, but the general thing is just: we don't want plugins…)

@lampholder lampholder added feature P3 Security P4 [OBSOLETE LABEL] Interesting — Not yet scheduled, will accept patches and removed P3 labels Feb 20, 2018
@lampholder
Copy link
Member

Admittedly I'm fairly uncomfortable triaging this one - please discuss vigorously if you think feature p4 security == "an interesting idea for a feature that possibly/probably has merit but we'd need to think more about before accepting a PR from the community and certainly isn't on our immediate roadmap to address" is not appropriate :)

@ara4n ara4n added P2 and removed P4 [OBSOLETE LABEL] Interesting — Not yet scheduled, will accept patches labels Feb 20, 2018
@ara4n
Copy link
Member

ara4n commented Feb 20, 2018

working towards a stronger CSP is certainly a good thing, and I don't think I experimented with embedding SVGs straight into the DOM at the time (which may also have other desirable perf benefits), so I think this is worth prioritising higher.

@oswee
Copy link

oswee commented Oct 7, 2018

Not sure how relevant this is there, but currently i am forced to use to solve my issue with appendChild. There is my issue from StackOverflow.

@germain-gg
Copy link
Contributor

germain-gg commented May 17, 2021

TintableSvg can now be deprecated. The appropriate way to achieve that in Element now would be to use mask.

As per this deprecation we can remove the roomColour setting too (#16278)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants