Make WordPress Core

Opened 3 months ago

Last modified 3 months ago

#61949 new defect (bug)

Do not call getimagesize() against non-existent files

Reported by: slaffik's profile slaFFik Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch
Focuses: Cc:

Description

When you call wp_get_attachment_image_src() against an attachment file that is not image but that does have the thumbnail attached to it OR when you call wp_getimagesize() directly - the later generates a PHP warning in certain cases because it retrieves image sizes for a file that does not exist.

The problem is indeed that a non-existent file path is passed, but there is no way to hijack it easily in certain cases.
While I'm trying to find a solution how to do all the filtering of wp_mime_type_icon and wp_get_attachment_image_src that I need, but I still think that a defensive mechanism can be implemented in WordPress itself.

There is no is_readable() check anywhere, it just all falls back to getimagesize( $filename ) with or without silencer (@) depending on the WP_DEBUG state.

My case is that in wp_get_attachment_image_src() from this call:

<?php
$src = wp_mime_type_icon( $attachment_id, '.svg' );

I return an image URL, which is correct and live. It's also used correctly in other areas of WordPress, hence the correct icon URL I filter there.

But then we have these calls:

<?php
$icon_dir = apply_filters( 'icon_dir', ABSPATH . WPINC . '/images/media' );
$src_file = $icon_dir . '/' . wp_basename( $src );

As you can see, it drops the path, just uses the file name, and appends it to the wp-includes path. Obviously, I can't filter the icon_dir filter as it has zero identifiable bits I can use to hijack the path and preserve the data I need.

And then it calls wp_getimagesize() without any filters of the src path before calling getimagesize().

It is possible to introduce new filters, rewrite the logic in wp_get_attachment_image_src() to fallback correctly, but IMO the easiest solution would be to just add is_readable() check like this at the top of the wp_getimagesize() function:

<?php
// We can't read what doesn't exist.
if ( ! is_readable( $filename ) ) {
        return false;
}

All warnings are gone, default failing false value preserved.

All the code and findings are as per WordPress 6.6.1.

Change History (1)

This ticket was mentioned in PR #7263 on WordPress/wordpress-develop by @slaFFik.


3 months ago
#1

  • Keywords has-patch added
Note: See TracTickets for help on using tickets.