10up / safe-svg

Enable SVG uploads and sanitize them to stop XML/SVG vulnerabilities in your WordPress website.
https://wordpress.org/plugins/safe-svg/
GNU General Public License v2.0
263 stars 31 forks source link

Fatal Error in Version 2.2.3, Calling member function attributes() on a Bool #184

Closed metashield-ie closed 5 months ago

metashield-ie commented 5 months ago

Describe the bug

In version 2.2.3, Line 598 of safe-svg.php calls the function simplexml_load_file($svg), which may return false. On line 600, it blindly calls $svg->attributes(); which results in a fatal error if simple xml has returned false;

Steps to Reproduce

Fatal Error after simplexml_load_file($svg). Version 2.2.3.

Screenshots, screen recording, code snippet

No response

Environment information

All

WordPress information

WordPress 6.4.3

Code of Conduct

ocean90 commented 5 months ago

Introduced by #154 which shipped in 2.2.3.

dkotter commented 5 months ago

Thanks for the report. I can see we lost an if statement in the refactoring done in #154. I've opened #186 to address this but would appreciate if you could test this, as I've not been able to reproduce the scenario where we have a valid svg file but simplexml_load_file can't load it. Thanks!

ocean90 commented 5 months ago

You should be able to reproduce this by uploading a SVG file and then deleting the file from the disk directly not using WordPress.

In our case we noticed this in a local dev environment where all uploads are loaded from remote so we don't need to copy them too.

dkotter commented 5 months ago

@ocean90 Thanks, took me a bit to reproduce but was finally able to (had to delete the file and ensure we didn't have height and width stored in the metadata). It seems PR #186 does fix this but would still love confirmation from a more real-world scenario.