[wp-trac] [WordPress Trac] #30377: wp_check_filetype is broken when checking urls with parameters

[WordPress Trac]

#30377: wp_check_filetype is broken when checking urls with parameters
-------------------------------------------------+-------------------------
 Reporter:  supercleanse                         |       Owner:  audrasjb
     Type:  enhancement                          |      Status:  reopened
 Priority:  normal                               |   Milestone:  6.0
Component:  Media                                |     Version:  4.0
 Severity:  normal                               |  Resolution:
 Keywords:  has-unit-tests needs-dev-note        |     Focuses:
  needs-patch                                    |
-------------------------------------------------+-------------------------

Comment (by dd32):

 Replying to [comment:62 Ruxton]:
 > With the knowledge of what dd32 just mentioned, it seems the viable
 solution outside of moving strip_fragment_from_url out of canonical (which
 is excessive) is to strip the fragment and beyond with strpos, etc. as the
 query string was.

 Using string functions to remove parts of a URL (both `?...` and `#...`)
 is a potential security issue here (and why it would've  been reverted
 prevously), as it's possible to cause it to strip more than anticipated
 and result in operating on the incorrect filename. You should really use
 `wp_parse_url()` instead too (It's kind of like parsing HTML with Regex,
 possible, but should be avoided).

 Additionally, since the parameter can be a Filename, Path, or URL, you
 need to restrict any URL handling to URLs only, not filenames.

 If a filename on disk ''looks like'' a url (You can have a filename on
 Linux that IS a url), you can't treat it as a URL either. If the filename
 came from an untrusted location, you can't treat it as a URL unless it's
 known to be intentionally a URL.

 Pulling out two comments from above:
  - [comment:19 Comment by me]: I'd personally prefer to add an explicit
 `wp_check_url_filetype()` function instead, one which extracts the actual
 filename from the URL and passes it along to `wp_check_filetype()` (if
 appropriate, or duplicates parts of it)
  - [comment:27 Comment by Helen]: Changing to enhancement, as
 `wp_check_filetype()` was never meant for URLs and the solution in this
 case is to introduce something else that does work for URLs.

 Even if the existing function is being passed URLs, it's not intentionally
 supposed to be used for URLs, and performing processing like this is
 doomed for failure.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/30377#comment:63>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform