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

Security issues #12

Closed
kimmobrunfeldt opened this issue Oct 5, 2017 · 4 comments
Closed

Security issues #12

kimmobrunfeldt opened this issue Oct 5, 2017 · 4 comments

Comments

@kimmobrunfeldt
Copy link
Member

It's easy to make Chrome display any file:// link. A couple of ways:

  • Redirect
  • window.location.href

Let's figure out if we could have a few ways in Puppeteer to block as much of these as possible. In any case, I'm quite confident that it's not possible to catch all of them. I would definitely recommend serving this API for "trusted" users, e.g. inside your organization.

@kimmobrunfeldt
Copy link
Member Author

A few ideas to overcome these issues from Hacker News:

You are using Chrome headless therefore you can use group policy to add "file://" to the URL blacklist; see http://www.chromium.org/administrators/policy-list-3#URLBlac...

  • cutcss

By default: if Headless Chrome hits a redirect to a file:// it returns: net::ERR_UNSAFE_REDIRECT
window.location.href = 'file:///' will return console error: "Not allowed to load local resource"

  • jotto

@kimmobrunfeldt
Copy link
Member Author

Additional discussion in Puppeteer repository: puppeteer/puppeteer#972.

I have a bit of mixed feelings with this. If we prevent rendering file:// urls in some cases, it may create a false sense of security. I'm a bit worried we won't be able to make the restrictions bulletproof. There are just so many ways to control the browser with JS. So I see two options:

  1. Remove restrictions for the url parameter, and allow any URLs. Then add an example of this security hole so that users of this API know it. This would then put pressure to secure the server environment instead.

  2. Try to patch all holes where user can access the file system. For example by preventing all requests except http/https, or disabling JS, or using page.setContent(), etc. I feel like this would be an eternal maintenance burden.

Also we have disabled the Chrome sandbox, which is a huge security issue: #4

@Janpot
Copy link

Janpot commented Feb 14, 2018

Additionally, you might want to prevent it from accessing private IP ranges in some use-cases. Like imagine you have a pdf rendering service or screenshotting service where users can submit a url to be rendered. Imagine they submit a url that resolve to private pages in your network. or urls that redirect to those pages, etc...

@kimmobrunfeldt
Copy link
Member Author

kimmobrunfeldt commented Aug 5, 2018

Summary

I'll close this issue. It serves more as an informational purpose and describing thought process.

In short: use the x-api-key authentication and/or run it in an isolated environment such as Heroku. Do not run it in an environment where you might end up exposing files from the host server or even other servers inside a private network.

Possible security "holes":

  • Allows attacker to expose file contents from the server via file:// urls.
  • Allows attacker to expose parts of private network, when hosted inside one and the API is publicly exposed.
  • Probably much more which I haven't thought about.

Why don't you fix them?

I think it's too big of a leap to try to cover all possible security holes and the better way is to just clearly communicate that you need to either use the x-api-key authentication or run it behind a firewall if you might have these issues. One option is to run it inside Heroku as the demo app does, and trust Heroku's isolation with their servers.

With the current efforts, it would be too ambitious for this project to try to cover all the security aspects.

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

No branches or pull requests

2 participants