Make WordPress Core

Opened 13 years ago

Closed 11 years ago

#17814 closed defect (bug) (fixed)

wp_load_image() attempts to load directories

Reported by: primehalo's profile primehalo Owned by: nacin's profile 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 13 years ago.
added is_file check to wp_load_image()
17814.patch (492 bytes) - added by SergeyBiryukov 12 years ago.
17814.2.diff (1.6 KB) - added by kirasong 11 years ago.
Updated patch for WP_Image_Editor.
17814.3.diff (1.5 KB) - added by kirasong 11 years ago.
Just check with is_file()
17814.test.diff (999 bytes) - added by kirasong 11 years ago.
Initial Unit Test.

Download all attachments as: .zip

Change History (14)

#1 @benkulbertis
13 years ago

  • Keywords has-patch added

@benkulbertis
13 years ago

added is_file check to wp_load_image()

#2 @scribu
13 years ago

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

#3 @nacin
12 years ago

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

#4 @SergeyBiryukov
12 years ago

  • Milestone changed from Future Release to 3.5

@kirasong
11 years ago

Updated patch for WP_Image_Editor.

#5 @kirasong
11 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

#6 follow-up: @scribu
11 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?

@kirasong
11 years ago

Just check with is_file()

#7 in reply to: ↑ 6 @kirasong
11 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.

@kirasong
11 years ago

Initial Unit Test.

#8 @kirasong
11 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.

#9 @nacin
11 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.