Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Content type hijack bug #31518

Closed
adam1010 opened this issue Feb 18, 2020 · 7 comments
Closed

Content type hijack bug #31518

adam1010 opened this issue Feb 18, 2020 · 7 comments

Comments

@adam1010
Copy link
Contributor

  • Laravel Version: 6.0 and up
  • PHP Version: 7.4.0
  • Database Driver & Version: Mysql 7

Description:

Using the Accept header on the request, a user/bot is able to change the Content-type to something that doesn't match the actual content. This may seem like it only affects that user and thus doesn't matter, but nginx and other proxies will cache this Content-type header and on my site I'm having raw HTML displayed to users because a bot filled my cache with XML Content-type headers for my HTML pages.

Steps To Reproduce:

  1. curl -I -H "Accept: text/xml" https://laravelshift.com/
    1a. Output has Content-Type: text/xml; even though that page is delivering HTML
  2. curl -I -H "Accept: application/json" https://laravelshift.com/
    2a. Output has Content-Type: application/json even though that page is delivering HTML

The Offending Code

The code that's doing this is located in Symfony\Component\HttpFoundation\Response in the prepare() method.
image

So Symfony is setting the Content-type to whatever the Request is asking for, regardless of what the type actually is. Laravel doesn't specify the Content-type when creating the Response for a regular HTML view. So if your controller has just return view('homepage') then Illuminate\Routing\Router::toResponse() runs without setting the Content Type to HTML

image

@jasonmccreary Figured I'd mention you here since I used your website as an example of being vulnerable to this. If you had a full page cache in front your site, you'd be seeing the same effect of bots causing regular users to see raw HTML displayed or error messages.

@jasonmccreary
Copy link
Contributor

@adam1010, I appreciate the tag, but it's important to note that this is not something specific to Shift, or even Laravel for that matter, but potentially all sites using this version of Symfony's Request object.

As such, I'd appreciate if you could use a generic example as this theoretically would behave the same for any Laravel 6 application, and again, not something specific to Shift.

In the meantime, I'll keep an eye out for any patches to ensure the response type matches.

@adam1010
Copy link
Contributor Author

@jasonmccreary Correct, this is in no way specific to Shift. I was just looking for site known to the Laravel community to demonstrate with. This may or may not affect Symfony applications, it depends how Symfony instantiates their Request object. If they always set the Content-Type then this "set a content type if one hasn't been set yet" code never runs.

The only reason Symfony is setting it here, is because Laravel didn't provide one and I guess they're trying to be helpful. I think Symfony could remove that block altogether but that doesn't mean Laravel can't protect itself by defaulting the Content-Type to text/html (or some other user-defined value) before Symfony is forced to guess in this bizarre way (ie instead of peeking at the content, it asks the requestor what content type it wished was being provided)

@X-Coder264
Copy link
Contributor

This issue is already reported on the Symfony repository: symfony/symfony#35694

@adam1010
Copy link
Contributor Author

thanks @X-Coder264 Seems like from the Symfony issue, when no content-type was set it defaulted to text/html previously (which is the correct behavior). So sounds like the fix here is for Laravel to pass in text/html as the content type header when creating the Symfony response inside the Router on line 749 (and not let Symfony have to do it for us and possibly screw it up)

$response = new Response($response);

@driesvints
Copy link
Member

@adam1010 with that solution how do you know the response to always be text/html? It could be a binary file, text/plain, literally anything.

@jasonmccreary
Copy link
Contributor

I agree, but I wonder if there is an opportunity to default this based on some safer assumptions. For example, either an ENV preset or when the view helper is used to default to text/html when not set.

Definitely can see this being a "gotcha" from both sides.

taylorotwell pushed a commit that referenced this issue Feb 20, 2020
This fixes issue #31518 where Laravel isn't setting a Content-Type and so Symfony is just setting it to whatever the request is asking for.  This is poisoning our cache, serving up our website HTML pages as "XML".
taylorotwell pushed a commit to illuminate/routing that referenced this issue Feb 20, 2020
This fixes issue laravel/framework#31518 where Laravel isn't setting a Content-Type and so Symfony is just setting it to whatever the request is asking for.  This is poisoning our cache, serving up our website HTML pages as "XML".
@driesvints
Copy link
Member

The pr for this got merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants