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

Control script execution in HTML preview #22103

Closed
mjbvz opened this issue Mar 6, 2017 · 4 comments
Closed

Control script execution in HTML preview #22103

mjbvz opened this issue Mar 6, 2017 · 4 comments
Assignees
Labels
plan-item VS Code - planned item for upcoming webview Webview issues
Milestone

Comments

@mjbvz
Copy link
Collaborator

mjbvz commented Mar 6, 2017

No description provided.

@mjbvz mjbvz added plan-item VS Code - planned item for upcoming webview Webview issues labels Mar 6, 2017
@mjbvz mjbvz added this to the March 2017 milestone Mar 6, 2017
@mjbvz mjbvz self-assigned this Mar 6, 2017
@kieferrm kieferrm mentioned this issue Mar 6, 2017
58 tasks
@formulahendry
Copy link
Member

Hi @mjbvz , just want to know the goal of this task. Does it mean script execution in HTML preview will be limited. So extensions like mssql (use Angular in HTML preview) will be impacted?

@mjbvz
Copy link
Collaborator Author

mjbvz commented Mar 15, 2017

@formulahendry We still plan on allowing extensions to execute scripts in the html preview, but this capability also brings up some important security concerns with the current implementation.

Problem

One potential security problem involving the preview is a script injection attack. Script injection is especially bad in the html preview case because the preview is served from the file:// origin, which means that any scripts can act in that origin. A classic exploit would be uploading a user's ~/passwords file.

Current Work

We've made some initial security improvements to the html preview and to the markdown preview that uses it. This includes disabling script execution by default in the markdown preview using a content security policy. However this does help any of the other extensions that are using the html preview. If they accidentally allow an attacker to inject unsanitized html content into the preview, they still open users up to attack.

Besides hardening the html preview itself, I'm working better documenting the html preview for extension authors. This will include a section on security best practices.

I'm also looking into adding an optional flag in the html preview command that would disable running in the file:// origin. This would likely be an opt-in flag, at least initially.

Longer Term

We want to ensure that even if arbitrary scripts can be injected into the html preview, nothing worse can happen to you than if you visited a website in Chrome. Some degree of security responsibility will still fall on extension authors, but we should certainly make exploits like the ~/passwords upload example difficult expose accidentally.

To achieve this, the first step would be to ensure that the html preview is not in the file:// origin and is instead served in it's own origin.

Benefits

  • Better security and isolation
  • Potentially finer grained control over local resource access

Problems

  • Breaks all local resource access, including images.
  • No clear migration plan (see below)

Work Required
We would like to allow the html preview to access some local resources (such as those shipped with an extension) but not others (such as your password file). To do this, I believe the best approach would be to register a new protocol using electron, such as vscode_resource://. This protocol would only allow access to specific resources on the system, such as all resources in your workspace and resources shipped with an extension.

How to migrate file:// resources to the new vscode_resource:// protocol is still unclear. Some work by extension authors would likely be required.

We may also want to have separate protocol for workspace resources and for extension resources. We may also want to allow certain resources from anywhere, such as images, while disallowing others

The other approach would be to spin up a simple web server and have the html preview use this. The web server would have to gate access to local resources properly and the migration for existing extensions is still not clear.

TL;DR

  • The html preview is a potential attack surface. We've made improvements to reduce the potential for this and exploiting the html preview still requires user action, such as downloading a malicious workspace and triggering a preview.
  • The impact of vulnerabilities in the html preview are amplified running in the file:// origin.
  • Truly fixing this while not breaking the world is hard. Many web standard security features are not fine grained when it comes to file:// resources.
  • If you maintain an extension that uses the html preview, it should continue working. Please continue testing your extension in insiders builds and let us know if it breaks because of any of the changes on our side. Also examine how you are constructing the html document for the preview and make sure you are not accidentally allowing script injection

@formulahendry
Copy link
Member

Cool~ Many thanks for your detailed information.

@mjbvz
Copy link
Collaborator Author

mjbvz commented Mar 22, 2017

As discussed, added security tips documentation with microsoft/vscode-docs#896 and c08ed01

Closing since documenting best practices was the main goal for this month

@mjbvz mjbvz closed this as completed Mar 22, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
plan-item VS Code - planned item for upcoming webview Webview issues
Projects
None yet
Development

No branches or pull requests

2 participants