WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

#14450 closed defect (bug) (fixed)

ms-files.php generates bad content-types for REQUEST_URI's with query string

Reported by: chrisjean Owned by:
Milestone: 3.0.2 Priority: normal
Severity: normal Version: 3.0
Component: Multisite Keywords: has-patch
Focuses: Cc:

Description

File requests of the type /subsite/files/style.css?ver=1 produce a bad content-type output. The request has to be for a multisite subsite as the request has to pass through wp-includes/ms-files.php.

The problem is caused by a call to wp_check_filetype that is passed an argument of $_SERVER['REQUEST_URI'] rather than $file. Since this variable contains the querystring of the request, it causes a failure of the regular expressions that are used in wp_check_filetype function.

This problem isn't present for all files as a fallback to mime_content_type is present, and that function is called with the correct argument.

Files with extensions of CSS are good examples of the problem as the mime_content_type function returns a type of text/plain in these cases. However, I have seen some servers that will return content-types such as "text/css?ver=1".

Attached is a patch that fixes this improper argument issue. It is built off of [15473].

Attachments (1)

14450.diff (842 bytes) - added by chrisbliss18 5 years ago.
Updated to replace other $_SERVERREQUEST_URI? instances with $file

Download all attachments as: .zip

Change History (10)

@chrisbliss185 years ago

Updated to replace other $_SERVERREQUEST_URI? instances with $file

comment:1 @chrisbliss185 years ago

I just updated the patch to replace two other instances of $_SERVER['REQUEST_URI'] with $file as these uses would also fail in the event that the request has a query string.

comment:2 @wpmuguru5 years ago

  • Milestone changed from Awaiting Review to 3.0.1

I didn't test, but that looks ok to me.

comment:3 @westi5 years ago

  • Cc westi added
  • Milestone changed from 3.0.1 to 3.0.2

Looks sane.
Would like to soak this in trunk before backporting to 3.0.x moving into 3.0.2 for that purpose.

comment:4 @westi5 years ago

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

(In [15749]) Ensure we don't generate incorrect content types when files are requested with query strings. Fixes #14450 props chrisbliss18.

comment:5 @nacin5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Re-opening to allow it to soak in trunk before potential backport.

comment:6 @hakre5 years ago

Security Note: w/o patch header injection, reported in #14594

+1 for backport beacuse of that.

comment:7 follow-up: @Denis-de-Bernardy5 years ago

Also, I closed #14594 even though it's not, technically, fully fixed yet... But since it was ignored for two months, I figured security is no longer a priority...

comment:8 in reply to: ↑ 7 @jane5 years ago

@Denis: There is absolutely no need for that tone. Security is always a priority.

comment:9 @nacin4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [16632]) Ensure we don't generate incorrect content types when files are requested with query strings. props chrisbliss18. fixes #14450 for the 3.0 branch.

Note: See TracTickets for help on using tickets.