rosell-dk / webp-express

Wordpress plugin for serving autogenerated WebP images instead of jpeg/png to browsers that supports WebP
GNU General Public License v3.0
229 stars 64 forks source link

fix(delete): sanity check filename when deleting file #583

Closed ColinFrick closed 1 year ago

ColinFrick commented 1 year ago

When wp_delete_file is called, the hook does not validate the given filename.

We had the problem, that a plugin called wp_delete_file with an empty string. This caused this hook to fail, because exif_imagetype used by the Mime class does not handle empty strings.

PHP Fatal error:  Uncaught ValueError: Path cannot be empty in /var/www/html/wp-includes/functions.php:3263
Stack trace:
#0 /var/www/html/wp-includes/functions.php(3263): exif_imagetype('')
#1 /var/www/html/wp-content/plugins/webp-express/lib/classes/Mime.php(17): wp_get_image_mime('')
#2 /var/www/html/wp-content/plugins/webp-express/lib/classes/Mime.php(46): WebPExpress\\Mime::getMimeTypeOfMedia('')
#3 /var/www/html/wp-content/plugins/webp-express/lib/classes/HandleDeleteFileHook.php(19): WebPExpress\\Mime::isOneOfTheseImageMimeTypes('', Array)
#4 /var/www/html/wp-includes/class-wp-hook.php(308): WebPExpress\\HandleDeleteFileHook::deleteAssociatedWebP('')
#5 /var/www/html/wp-includes/plugin.php(205): WP_Hook->apply_filters('', Array)
#6 /var/www/html/wp-includes/functions.php(7322): apply_filters('wp_delete_file', '')
#7 ...

I call SanityCheck::absPathExistsAndIsFileInDocRoot at the beginning of the function, because HandleUploadHooks validates it the same way.

rosell-dk commented 1 year ago

@ColinFrick: Which plugin called wp_delete_file with an empty string? I'd like to know because I will only trigger a release on wordpress.org if it is a popular plugin

ColinFrick commented 1 year ago

Hey @rosell-dk Thanks for the merge

The plugin Lumise called wp_delete_file with an empty string: https://lumise.com/ https://codecanyon.net/item/lumise-product-designer-woocommerce-wordpress/21222684