WordPress.org

Make WordPress Core

Opened 9 years ago

Last modified 5 months ago

#17061 accepted defect (bug)

media_sideload_image() allows upload of 301-redirected non-images

Reported by: Coolkevman Owned by: chriscct7
Milestone: Priority: normal
Severity: normal Version: 3.1.1
Component: Media Keywords: needs-patch
Focuses: Cc:
PR Number:

Description

When you give media_sideload_image() URLs that are not images, an error is raised.

If for example I do:

media_sideload_image("http://google.com", $post_id, $img_desc);

then I get this error:

Sorry, this file type is not permitted for security reasons.

And this is absolutely normal and expected.

But there is a case when media_sideload_image() do not detect non-images. This case is when the URL given to the function looks like an image but is redirected by Apache to another place.

For example, on my server, this URL:

http://coolcavemen.com/e107_plugins/autogallery/Gallery/default.jpg

redirects to:

http://coolcavemen.com/photos/

Now if in some PHP code I do:

media_sideload_image("http://coolcavemen.com/e107_plugins/autogallery/Gallery/default.jpg", $post_id, $img_desc);

then no error is raised and I end up with the HTML served at http://coolcavemen.com/photos/ being uploaded to my WordPress site as-is:

kevin@kev-laptop$ file ./wp-content/uploads/2011/04/default.jpg
./wp-content/uploads/2011/04/default.jpg: HTML document text

Of course this upload appears broken in the media manager, as you can see in this screenshot: http://twitpic.com/4hlyks

Attachments (1)

17061.patch (961 bytes) - added by kurtpayne 8 years ago.
Attempts to get MIME info and block non-image sideloads. Uses fileinfo extension, falls back to mime_content_type(), gracefully degrades to current behavior if nothing is available.

Download all attachments as: .zip

Change History (5)

#1 @kurtpayne
8 years ago

  • Cc kpayne@… added
  • Component changed from HTTP to Media
  • Keywords needs-patch added

I can confirm this still happens with 3.3 beta 4. The problem is that the type is sniffed from the first URL instead of the content-type of the file that was downloaded.

Related #18730.

@kurtpayne
8 years ago

Attempts to get MIME info and block non-image sideloads. Uses fileinfo extension, falls back to mime_content_type(), gracefully degrades to current behavior if nothing is available.

#2 @mikeschroder
7 years ago

  • Cc mike.schroder@… added

#3 @ericlewis
5 years ago

Should this logic go further upstream, to media_handle_sideload()?

#4 @chriscct7
4 years ago

  • Owner set to chriscct7
  • Status changed from new to accepted
Note: See TracTickets for help on using tickets.