Make WordPress Core

Opened 10 years ago

Last modified 7 days ago

#36273 new defect (bug)

update_attached_file() on Windows will result in invalid image path when using native Windows directory separators

Reported by: whissi's profile Whissi Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.4.2
Component: Media Keywords: needs-unit-tests has-patch
Focuses: Cc:

Description

Calling update_attached_file( $image->ID, $file ); on platforms like Windows can be really bad if $file was normalized/validated using PHP's realpath() function:

<?php
$id = 123; // Image
$image = get_post( $id );

if ( is_null( $image) || !preg_match( '!^image/!', get_post_mime_type( $image ) ) ) {
        die( sprintf( "Invalid image id (#%s)", $id ) );
}

// Easy case: Let's imagine we can get the path from meta data
$file = get_attached_file( $image->ID );

// Well, in real world you could have created the path manually...
// The only important thing to know is, that we call "realpath()" which will
// convert any directory separator into the native directory separator:
// Linux will end with /dir/subdir/basename.jpg
// Windows will end with C:\Dir\Subdir\basename.jpg
$file = realpath( $file );

// Again, this is just a demo, for real world cases see plugins like "Force Regenerate Thumbnails"
// But this is a valid API call:
update_attached_file( $image->ID, $file );

// On Windows this will result in an update statement like
//   UPDATE `postmeta` SET `meta_value` = 'C:WWWSitesdemohtdocswordpresswp-contentuploads201603example.jpg' WHERE `post_id` = 123 AND `meta_key` = '_wp_attached_file'
// when $file was set to "C:\WWW\Sites\demo\htdocs\wordpress\wp-content\uploads\2016\03\example.jpg"
// Now imagine a plugin which is re-generating thumbnails :]

// The problem is
//   $meta_value = wp_unslash($meta_value);
// in wp-includes/meta.php update_metadata().

When using update_attached_file() we should make sure that update_metadata() don't update the path value to an invalid value...

PS: After you updated all image paths to an invalid value, the media library won't work anymore:

[18-Mar-2016 07:31:10 UTC] PHP Warning:  file_exists() expects parameter 1 to be a valid path, string given in C:\WWW\Sites\demo\htdocs\wordpress\wp-includes\media.php on line 3063

Change History (4)

#1 @SergeyBiryukov
10 years ago

  • Component changed from General to Media
  • Keywords needs-patch needs-unit-tests added

Sounds like update_attached_file() should pass $file through wp_normalize_path().

This ticket was mentioned in Slack in #gdpr-compliance by allendav. View the logs.


8 years ago

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


7 days ago
#3

  • Keywords has-patch added; needs-patch removed

On Windows, PHP's realpath() returns paths with native backslash directory separators (e.g. C:\WWW\Sites\demo\wp-content\uploads\2016\03\example.jpg). When these paths are passed to update_attached_file(), the backslashes are stripped by wp_unslash() inside update_metadata(), resulting in an invalid path being stored in postmeta (e.g. C:WWWSitesdemowp-contentuploads201603example.jpg).

This breaks the media library entirely on Windows, as file_exists() and other file functions fail on the corrupted path.

The fix passes $file through wp_normalize_path() after _wp_relative_upload_path() converts it to a relative path, ensuring all backslashes are converted to forward slashes before the value is stored in the database.

Trac ticket: https://core.trac.wordpress.org/ticket/36273

## Use of AI Tools

#4 @shreya0shrivastava
7 days ago

Hi, I was able to reproduce this issue on 7.1-alpha-62161-src and confirmed the bug is still present.

The fix passes $file through wp_normalize_path() after _wp_relative_upload_path(),
which converts all backslashes to forward slashes before the value is stored
in postmeta, preventing wp_unslash() from corrupting the path.

Submitted a patch. Happy to make any changes based on feedback!

Note: See TracTickets for help on using tickets.