Allow SVGs to be uploaded on all admin pages #240
Conversation
There was a problem hiding this comment.
Thanks for the PR @stormrockwell. While I understand the desire to upload SVGs in more contexts, the changes made in #228 were done to fix a security vulnerability and as such, we're unlikely to change the behavior introduced there.
The short version here is that we only want to allow SVGs to be uploaded in contexts where we can be sure the SVG is passing through our sanitization methods. Otherwise someone could upload an insecure SVG and that could then possibly execute unwanted code. So allowing SVGs to be uploaded in any admin context could allow SVGs to bypass sanitization.
|
@dkotter I understand the reasoning, but it's a bit of a bummer. Has anyone looked into if there's a way to ensure SVGs are sanitized in additional contexts? If not, I can try to look into it at a later date |
Our main goal was to ensure SVGs uploaded in traditional contexts pass through sanitization so not sure much time was spent looking at other contexts. Definitely interested though in expanding this so if you have time/interest to dive further into that, would be greatly appreciated |
|
@dkotter Wanted to post an update. I wasn't able to figure out how to bypass the sanitization after the previous security patch adding the wp_handle_sideload_prefilter hook. If someone could provide additional context on an additional hole with hooking in at the admin_init level I can explore more |
Description of the Change
#228
In 2.3.0, there was a breaking change for us, which prevented the addition of SVGs outside of posts or media library. I need the ability to add SVGs on admin pages/tools/terms/etc.
I've made it a lot broader by instead hooking into admin_init, and all of the tests seem to pass except one that doesn't run on develop either for me, "Admin can add SVG block to a post."
I initially went with adding hooks for load-admin.php/load-edit-tags.php/etc, but I didn't see an issue broadening it to all of the admin interfaces, which might be my ignorance of this project. If there are better hooks we can use to cover every case, let me know, and I will make the changes!
How to test the Change
Try adding SVGs to option pages, often added by plugins/themes.
Changelog Entry
Fixed - Bug that prevented admins from uploading SVGs on admin pages outside of the post and media library pages.
Credits
Just me and whomever helps if we change the hooks!
Checklist: