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

Nightmare Security Checklist #1388

Closed
matthewmueller opened this issue Feb 28, 2018 · 3 comments
Closed

Nightmare Security Checklist #1388

matthewmueller opened this issue Feb 28, 2018 · 3 comments

Comments

@matthewmueller
Copy link
Contributor

matthewmueller commented Feb 28, 2018

This checklist explains what nightmare has implemented from the electron security guidelines and why.

⛔️ Only load secure content

We can't do this because some websites you'll want to visit with nightmare are insecure (e.g. http)

✅ Disable the Node.js integration in all renderers that display remote content

This has been disabled to prevent any website executing node.js code on your computer.

⛔️ Enable context isolation in all renderers that display remote content

We don't do this yet, but we're very careful which data can be sent through the IPC.

We should be able to enable this once we refactor .evaluate(src) to use promises. More info here: #1387

⛔️ Use ses.setPermissionRequestHandler() in all sessions that load remote content

We don't use a default permission request handler, but it would be easy to create one with custom actions.

The concern here is that a potentially malicious website could ask for your location (or one of the other permissions), and electron automatically accepts the permissions by default

✅ Do not disable webSecurity

We don't disable web security

⛔️ Define a Content-Security-Policy and use restrictive rules (i.e. script-src 'self')

Unfortunately, this would cause some websites to stop working.

⛔️ Override and disable eval, which allows strings to be executed as code.

Unfortunately, this would cause some websites to stop working.

✅ Do not set allowRunningInsecureContent to true

We don't allow running insecure content

✅ Do not enable experimental features

We don't enable experimental features

✅ Do not use blinkFeatures

We don't use blink features

✅ WebViews: Do not use allowpopups

We don't use webviews.

✅ WebViews: Verify the options and params of all tags

We don't use webviews.

@glyph
Copy link

glyph commented Aug 15, 2018

✅ Do not enable experimental features

We don't disable experimental features

Hopefully one of these words is backwards?

@matthewmueller
Copy link
Contributor Author

@glyph good catch! updated

@glyph
Copy link

glyph commented Aug 17, 2018

Thanks!

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

No branches or pull requests

2 participants