Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Don't serve user-uploaded files with correct mime/content type #2877

Open
rugk opened this issue Feb 15, 2018 · 3 comments
Open

Don't serve user-uploaded files with correct mime/content type #2877

rugk opened this issue Feb 15, 2018 · 3 comments
Labels
A-Media-Repository Uploading, downloading images and video, thumbnailing Security T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.

Comments

@rugk
Copy link

rugk commented Feb 15, 2018

Yes this tackles your known problem. So it's good you use some CSP there to prevent the execution of JavaScript, but that is not nearly enough.

And no, I don't accept that you tell users to use another server/domain. That may be an additional mitigation, but you cannot require each user to do so and it is just not a good idea to think that each user could do so.

Also it e.g. has a phishing risk. When I click on such a file in Riot, it just directs me to the new server and displays that file. That means if the HTML file just emulates the Riot login screen, some users may be tricked with that. (And as the website opens in the same window it is not the same as if I just click an arbitrary external link, which opens in a new tab.)
Nobody needs to display a website when one sends a HTML file! These may be devs or so and they can just open it locally… (it also makes less sense when JS is broken anyway, thanks to the CSP.)

That also tackles you spec, so…
But in Riot and everywhere users also just use this thing to download files. So why not simply instruct the browser for a download? Set application/octet-stream as the content type. That's a very simple solution.

As for JPGs showing inline in a browser, that's also no problem. One AJAX call to download the file and then you can load it via data: anyway. (I think, that may be done when encryption is enabled, anyway.)

So the spec requiring Content-Type to be correct, is not only useless, but just really harmful.

@richvdh
Copy link
Member

richvdh commented Feb 22, 2018

And no, I don't accept that you tell users to use another server/domain. That may be an additional mitigation, but you cannot require each user to do so and it is just not a good idea to think that each user could do so.

We recommend that people do, but it's not a hard requirement.

One AJAX call to download the file and then you can load it via data: anyway. (I think, that may be done when encryption is enabled, anyway.)

It sounds very expensive to do that for every avatar image.

@rugk
Copy link
Author

rugk commented Feb 22, 2018

Okay, then don't do it for avatars. Avatars can just be served as PNG/JPG or so, when you restrict the content type to that, this is okay. (SVG may be problematic though)
The big issue is there for HTMl files and stuff like that.

Also I don't really know whether an AJAX call and data: display is really more expensive than a img tag. Both can (and probably should) be cached.

@sbuller
Copy link

sbuller commented Oct 16, 2018

I'm a little confused by the situation with regards to the security note. As I understand, I want to use my main domain for hosting the user content, but I also have to use a different domain for the homeserver since it blindly serves up user content. I'm not sure how to evaluate the security mitigations that are in place. Doesn't it make sense to do like github did in https://developer.github.com/changes/2014-04-25-user-content-security/, and allow the user content to be served from a different domain?

As I'm understanding things, I want my client api to be hosted at <example.com/_matrix> so that my matrix ids are of the form @user:example.com, but then the homeserver ends up hosting user content from the api endpoint. Isn't this a problem, not just for the matrix client, but also for everything else that I'm hosting on example.com?

I would happily reverse-proxy usercontent.example.com to an additional endpoint, or provide access via a different port. As I read the Same Origin Policy, either of these should offer protection, since I'm not using document.domain in anything I'm serving.

All that said, I think the idea of restricting the mime type on user content makes sense. Perhaps a whitelist makes sense along with defaulting to 'application/octet-stream' and setting "X-Content-Type-Options: nosniff".

@reivilibre reivilibre added A-Media-Repository Uploading, downloading images and video, thumbnailing Security T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. labels May 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Media-Repository Uploading, downloading images and video, thumbnailing Security T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.
Projects
None yet
Development

No branches or pull requests

4 participants