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

Should use secure mimetypes instead of generic ones for download routes #347

Closed
LukasReschke opened this issue Sep 15, 2015 · 8 comments
Closed
Assignees
Labels
Milestone

Comments

@LukasReschke
Copy link
Member

See owncloud/core#10938 for reasoning

GET /master/index.php/apps/gallery/files/download/3?c=b2a1325df3e7968027e36a3fe7bf8be8&requesttoken=8947b86c939d724cd60efe4a165e9129f7f7b155b41e0ea1095c84028d336537%7CyRbLmmgx%2FB7Jkpoz%7Cf80eb122cbb7f40379bdf45a3792641787bcd5d6410ce0463045a0482729ce86cc71b6f4837832488a9d7143280a93a1ca3acb0805a21b878092ed0fad4f76a0%3A4NlU4wom3c HTTP/1.1
Host: localhost
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:40.0) Gecko/20100101 Firefox/40.0
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate
Cookie: ocnqhvnh1crb=402c3ing7r53kn1dgp225i7p27; oc_sessionPassphrase=vx00ticTJMER1O8oemQbj5FK3pMGIXkUwuqNuqjjpK%2BK1jY1hiye76hB2fsoBTfarqIB3VhW6AMU0pdrIVs1wI%2FwMnygUJC%2Fwm07OGRU1gcBQU50N6%2FgNr%2FyeGfInsun
Connection: keep-alive
HTTP/1.1 200 OK
Date: Tue, 15 Sep 2015 15:34:04 GMT
Server: Apache/2.4.16 (Unix) PHP/5.6.11
X-Powered-By: PHP/5.6.11
Expires: Thu, 19 Nov 1981 08:52:00 GMT
Cache-Control: no-cache, must-revalidate
Pragma: no-cache
Content-Security-Policy: default-src 'none';script-src 'self' 'unsafe-eval';style-src 'self' 'unsafe-inline';img-src 'self' data:;font-src 'self';connect-src 'self';media-src 'self'
Content-Disposition: attachment; filename*=UTF-8''welcome.js; filename="welcome.js"
Content-Length: 163
X-Content-Type-Options: nosniff
X-XSS-Protection: 1; mode=block
X-Robots-Tag: none
X-Frame-Options: SAMEORIGIN
Keep-Alive: timeout=5, max=100
Connection: Keep-Alive
Content-Type: application/javascript; charset=utf-8

Welcome to your ownCloud account!

This is just an example file for developers and git users. 
The packaged and released versions will come with better examples.

This should use a secure mimetype alternative instead of application/javascript. Such as via \OC::$server->getMimeTypeDetector()->getSecureMimeType().

@LukasReschke
Copy link
Member Author

Or alternative is to add a check if the item in question is in fact a image/* mimetype, also add a check to verify if SVG support is enabled.

That said here one needs to ensure that there is no potential race condition vector :)

(User adds file as image, user requests file and check succeeds since mimetype is an image, user modifies file to a JS file, JS file is served)

@oparoz
Copy link
Contributor

oparoz commented Sep 15, 2015

Should be a quick fix as there is already a method which gives me the list of supported media types:
https://github.com/owncloud/gallery/blob/master/service/previewservice.php#L79-L107

AFAIK, there would be only one extra step required, which is to change the type for SVGs.

Sounds like a plan?

@LukasReschke
Copy link
Member Author

👍

@oparoz oparoz added the ready label Sep 17, 2015
@oparoz oparoz added this to the 8.2-current milestone Sep 17, 2015
@oparoz oparoz self-assigned this Sep 17, 2015
@oparoz oparoz added in progress and removed ready labels Sep 18, 2015
@oparoz
Copy link
Contributor

oparoz commented Sep 18, 2015

It's more complicated than I initially thought, because we still need to show the downloaded SVG in the browser for the slideshow per example. It's not just to store it away.

@LukasReschke
Copy link
Member Author

It's more complicated than I initially thought, because we still need to show the downloaded SVG in the browser for the slideshow per example. It's not just to store it away.

I'd do the following:

-> If SVG support is enabled use SVG as mimetype it was anyways advertised as potentially insecure. For everything else use the secure mimetype.
-> If SVG support is not enabled check for image/* and use the secure mimetype

@oparoz
Copy link
Contributor

oparoz commented Sep 18, 2015

Yeah, I'll try that.

I was successful in using base64, but it doesn't work for the slideshow on the Files side:
Content Security Policy: The page's settings blocked the loading of a resource at data:image/svg+xml;base64,PD94bWwgdmVyc2lvb...

@oparoz
Copy link
Contributor

oparoz commented Sep 18, 2015

Native SVG support can only be determined by parsing the config every time a download/preview is requested, so I'll just add an extra parameter to the download preview endpoint which defaults to false.

I'll also restrict the type of files which can be served from the endpoints. It will be limited to the media types Gallery supports.

@oparoz
Copy link
Contributor

oparoz commented Sep 18, 2015

  1. Always send text/plain for SVG at the download endpoints
  2. Always send text/plain for SVG at the internal and public preview endpoints (non-API)
  3. Only allow the SVG media type for the preview endpoint in the API if explicitly asked for
  4. Only serve files of the media types we support

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

No branches or pull requests

2 participants