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.

Sign up for GitHub

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

Jump to bottom

CSP doesn't work for scripts inside inline <webview> tags? #3921

Closed
seanchas116 opened this issue Dec 25, 2015 · 9 comments
Closed

CSP doesn't work for scripts inside inline <webview> tags? #3921

seanchas116 opened this issue Dec 25, 2015 · 9 comments

Comments

@seanchas116
Copy link
Contributor

Reference: Electronでアプリを書く場合は、気合いと根性でXSSを発生させないようにしなければならない。 - 葉っぱ日記 (Japanese)

Content Security Policy doesn't work for scripts inside <webview> tags?

I think it's dangerous because CSP cannot prevent XSS attacks using <webview> tags and allows access to any Node APIs.

Example

  • Inline script inside <script> doesn't run
  • Script in <webview>'s src attribute runs (and any Node APIs can be used in it even if outer BrowserWindows is nodeIntegration disabled)
<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="Content-Security-Policy" content="default-src 'self';">
  </head>
  <body>
    <script>
      alert("hoge");
    </script>
    <webview nodeintegration src="data:text/html,<script>require('child_process').exec('atom package.json')</script>"></webview>
  </body>
</html>
new BrowserWindow({width: 600, height: 400, webPreferences: {nodeIntegration: false}});

Environment: Electron 0.36.1 and OS X 10.11.2
@zcbenz
Copy link
Contributor

zcbenz commented Dec 28, 2015

The src of webview is loaded in the webview's page instead of current page, so current page's CSP settings won't affect webview's src.

@zcbenz zcbenz closed this as completed Dec 28, 2015
@seanchas116
Copy link
Contributor Author

@zcbenz Is it OK to allow this sort of code injection?

@zcbenz
Copy link
Contributor

zcbenz commented Dec 28, 2015

If users can inject a webview tag then it won't be safe anyway, they would be able to load arbitrary content and run arbitrary code. There is a similar discussion: #1753.

@seanchas116 seanchas116 mentioned this issue Dec 28, 2015
Closed
@anaisbetts
Copy link
Contributor

@zcbenz WebView tag should probably just be flat-out disabled if node integration is disabled

@seanchas116
Copy link
Contributor Author

I think <webview> tag should be disabled and WebView should be created only in preload scripts if Node integrations is disabled.

@zcbenz zcbenz mentioned this issue Dec 29, 2015
Closed
@zcbenz
Copy link
Contributor

zcbenz commented Dec 29, 2015

Disabling webview when node integration is off could have compatibility issues with existing apps, I think we can provide an option to disable webview. I have create #3943 for this.

@anaisbetts
Copy link
Contributor

Disabling webview when node integration is off could have compatibility issues with existing apps

You're not wrong, but those apps are also disabling node integration for security reasons, and this bug means that all remote content hosted in these windows effectively has full access to the machine, this is a security issue serious enough imho to warrant a breaking change

As an equally-secure yet perhaps not as breaking of a change, we could ignore nodeintegration if the parent doesn't have it enabled

@zcbenz
Copy link
Contributor

zcbenz commented Dec 29, 2015

You're not wrong, but those apps are also disabling node integration for security reasons, and this bug means that all remote content hosted in these windows effectively has full access to the machine, this is a security issue serious enough imho to warrant a breaking change

Sounds quite reasonable to me 👍.

@wearhere wearhere mentioned this issue Jan 8, 2016
Closed
@anghene anghene mentioned this issue Apr 26, 2016
Closed
@Nantris
Copy link
Contributor

Nantris commented May 10, 2020

The src of webview is loaded in the webview's page instead of current page, so current page's CSP settings won't affect webview's src.

@zcbenz, this doesn't seem to be true in Electron 8? Is there any way to use the webview without it having the BrowserWindow's CSP applied?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@anaisbetts @zcbenz @seanchas116 @Nantris