Disable webview when node integration is off #3943

Closed
zcbenz opened this Issue Dec 29, 2015 · 11 comments

Comments

Projects
None yet
7 participants
@zcbenz
Contributor

zcbenz commented Dec 29, 2015

Currently webview tag is available even when node integration is off, this can introduce security problems since webview can be used to execute arbitrary code.

Refs #3921.

@zcbenz zcbenz changed the title from Add an option to disable webview to Disable webview when node integration is off Dec 29, 2015

@xywz

This comment has been minimized.

Show comment
Hide comment
@xywz

xywz Dec 29, 2015

Will the preload script still be able to create webviews?

xywz commented Dec 29, 2015

Will the preload script still be able to create webviews?

@seanchas116

This comment has been minimized.

Show comment
Hide comment
@seanchas116

seanchas116 Dec 29, 2015

Contributor

Will the preload script still be able to create webviews?

How about adding new WebView modules or something to create webview element without <webview> tag? It can be used in preload scripts and safe from injection.

Example:

const WebView = require("electron").WebView;
const webView = new WebView({src: ...});
document.body.appendChild(webView.element);
Contributor

seanchas116 commented Dec 29, 2015

Will the preload script still be able to create webviews?

How about adding new WebView modules or something to create webview element without <webview> tag? It can be used in preload scripts and safe from injection.

Example:

const WebView = require("electron").WebView;
const webView = new WebView({src: ...});
document.body.appendChild(webView.element);
@wearhere

This comment has been minimized.

Show comment
Hide comment
@wearhere

wearhere Jan 8, 2016

Contributor

Rather than disable the webview tag altogether, why not just prevent Node integration from being re-enabled on the webview? Related: #4026

Contributor

wearhere commented Jan 8, 2016

Rather than disable the webview tag altogether, why not just prevent Node integration from being re-enabled on the webview? Related: #4026

@xywz

This comment has been minimized.

Show comment
Hide comment
@xywz

xywz Apr 2, 2016

This is a separate issue that should remain open. Arbitrary webpages can still use webviews to bypass CSP, get access to the entire webContents API, and do many other potentially damaging actions.

xywz commented Apr 2, 2016

This is a separate issue that should remain open. Arbitrary webpages can still use webviews to bypass CSP, get access to the entire webContents API, and do many other potentially damaging actions.

@kevinsawicki kevinsawicki reopened this Apr 4, 2016

@bridiver

This comment has been minimized.

Show comment
Hide comment
@bridiver

bridiver Apr 5, 2016

Contributor

after our release this week I'm going to resolve a few backwards compatibility issues and put together PRs for the electron work we've done. We sandboxed and completely removed node from renderer processes that have node integration disabled. Webviews are not available, but preload script capability is available through extension content scripts (including ipcRenderer)

Contributor

bridiver commented Apr 5, 2016

after our release this week I'm going to resolve a few backwards compatibility issues and put together PRs for the electron work we've done. We sandboxed and completely removed node from renderer processes that have node integration disabled. Webviews are not available, but preload script capability is available through extension content scripts (including ipcRenderer)

@laramies

This comment has been minimized.

Show comment
Hide comment
@laramies

laramies Apr 21, 2016

@bridiver I am interested in the sandboxing and removing node from renderer processes work. Any chance to do the PRs?

@bridiver I am interested in the sandboxing and removing node from renderer processes work. Any chance to do the PRs?

@laramies

This comment has been minimized.

Show comment
Hide comment
@laramies

laramies May 12, 2016

Any progress on fixing this issue? Or porting the changes from Brave? Thanks in advance

Any progress on fixing this issue? Or porting the changes from Brave? Thanks in advance

@bridiver

This comment has been minimized.

Show comment
Hide comment
@bridiver

bridiver May 12, 2016

Contributor

We have everything working, but our nodeless renderer processes use content scripts as a replacement for the preload script. It's fairly simple to remove node and add the sandbox, but without extensions you'll be left without preload script support. I've had some discussions with @zcbenz about extensions and he feels pretty strongly that they should not be included in Electron, so I'm not sure if our changes will be helpful.

Contributor

bridiver commented May 12, 2016

We have everything working, but our nodeless renderer processes use content scripts as a replacement for the preload script. It's fairly simple to remove node and add the sandbox, but without extensions you'll be left without preload script support. I've had some discussions with @zcbenz about extensions and he feels pretty strongly that they should not be included in Electron, so I'm not sure if our changes will be helpful.

@xywz

This comment has been minimized.

Show comment
Hide comment
@xywz

xywz May 18, 2016

@zcbenz can there be an option to enable webviews? This breaking change weakens the security of apps that use webviews with preload scripts because now all scripts in the page will have access to node instead of just one.

xywz commented May 18, 2016

@zcbenz can there be an option to enable webviews? This breaking change weakens the security of apps that use webviews with preload scripts because now all scripts in the page will have access to node instead of just one.

@zcbenz

This comment has been minimized.

Show comment
Hide comment
@zcbenz

zcbenz May 18, 2016

Contributor

@xywz I'm not sure I get what you meant, the change was to disable webview itself, instead of node integration in webview.

Contributor

zcbenz commented May 18, 2016

@xywz I'm not sure I get what you meant, the change was to disable webview itself, instead of node integration in webview.

@xywz

This comment has been minimized.

Show comment
Hide comment
@xywz

xywz Jun 10, 2016

@zcbenz currently node integration is disabled for the window and webviews are used. The code that needs access to node is in a preload script and all other code runs in the standard browser context. Updating will break this setup unless there is an option to enable webviews.

xywz commented Jun 10, 2016

@zcbenz currently node integration is disabled for the window and webviews are used. The code that needs access to node is in a preload script and all other code runs in the standard browser context. Updating will break this setup unless there is an option to enable webviews.

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