WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#28208 closed defect (bug) (fixed)

in wp_handle_upload setting the overwrite `test_upload` to false won't work for move_uploaded_file

Reported by: bassjobsen Owned by: wonderboymusic
Milestone: 4.1 Priority: normal
Severity: normal Version: 3.9.1
Component: Upload Keywords: has-patch
Focuses: Cc:

Description (last modified by dd32)

move_uploaded_file() also checks is_uploaded_file so if you skip is_uploaded_file by setting test_upload to false you can still upload your file cause move_uploaded_file() blocks it.

Possible solution

replace

 if ( false === @ move_uploaded_file( $file['tmp_name'], $new_file ) ) {

with:

if ( (false === $test_upload && false === @ rename( $file['tmp_name'], $new_file ))	|| false === @ move_uploaded_file( $file['tmp_name'], $new_file ) ) {

Attachments (1)

28208.diff (1.1 KB) - added by wonderboymusic 5 years ago.

Download all attachments as: .zip

Change History (5)

#1 @dd32
5 years ago

  • Component changed from Filesystem API to Upload
  • Description modified (diff)

@wonderboymusic
5 years ago

#2 @wonderboymusic
5 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 4.1

$test_upload is useless and just causes errors later in the function if the check is skipped. I suggest 28208.diff

This ticket was mentioned in Slack in #core by wonderboymusic. View the logs.


5 years ago

#4 @wonderboymusic
5 years ago

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

In 30076:

In _wp_handle_upload(), if test_upload is set to false in the array of dangerous overrides that the function allows, the only thing that happens when an upload fails is more potential breakage.

$test_uploaded_file lets is know if $file['tmp_name'] exists, which allows to exit with an error, instead of continuing to attempt to move the file.

$test_upload override is now a noop.

Fixes #28208.

Note: See TracTickets for help on using tickets.