WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 3 years ago

#17814 closed defect (bug) (fixed)

wp_load_image() attempts to load directories

Reported by: primehalo Owned by: nacin
Milestone: 3.5 Priority: normal
Severity: minor Version: 3.1.3
Component: Media Keywords: has-patch
Focuses: Cc:

Description

The wp_load_image() function in /wp-includes/media.php does a file_exists() check on the $file parameter, but that will return TRUE for both files and directories. There is no check to make sure $file is actually a file instead of a directory.

Attachments (5)

media-17814.php.patch (859 bytes) - added by benkulbertis 4 years ago.
added is_file check to wp_load_image()
17814.patch (492 bytes) - added by SergeyBiryukov 3 years ago.
17814.2.diff (1.6 KB) - added by DH-Shredder 3 years ago.
Updated patch for WP_Image_Editor.
17814.3.diff (1.5 KB) - added by DH-Shredder 3 years ago.
Just check with is_file()
17814.test.diff (999 bytes) - added by DH-Shredder 3 years ago.
Initial Unit Test.

Download all attachments as: .zip

Change History (14)

comment:1 @benkulbertis4 years ago

  • Keywords has-patch added

@benkulbertis4 years ago

added is_file check to wp_load_image()

comment:2 @scribu4 years ago

  • Component changed from General to Media
  • Milestone changed from Awaiting Review to Future Release

comment:3 @nacin3 years ago

We can likely combine this into the ! file_exists() check above it, and use the same string.

@SergeyBiryukov3 years ago

comment:4 @SergeyBiryukov3 years ago

  • Milestone changed from Future Release to 3.5

@DH-Shredder3 years ago

Updated patch for WP_Image_Editor.

comment:5 @DH-Shredder3 years ago

Updated patch to include WP_Image_Editor, and the file-name change for wp_load_image (since the function is now deprecated).

Related: #6821

comment:6 follow-up: @scribu3 years ago

  • Summary changed from wp_load_image() to wp_load_image() attempts to load directories

Couldn't we just replace the file_exists() check with is_file() instead of calling both?

@DH-Shredder3 years ago

Just check with is_file()

comment:7 in reply to: ↑ 6 @DH-Shredder3 years ago

Replying to scribu:

Couldn't we just replace the file_exists() check with is_file() instead of calling both?

You're right, looks like we can!
Went ahead and updated the patch. Will write up a quick unit test for this later today, if no-one else gets to it first.

@DH-Shredder3 years ago

Initial Unit Test.

comment:8 @DH-Shredder3 years ago

Initial unit test attached. At the moment, errors upon wp_load_image() failure. Going to see if there's a better way to do this.

comment:9 @nacin3 years ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 22463:

In WP_Image_Editor / wp_load_image(), use is_file() rather than file_exists() so we do not accidentally load a directory.

props benkulbertis, DH-Shredder, scribu.
fixes #17814.

Note: See TracTickets for help on using tickets.