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

Help webview extensions add a Content Security Policy #79340

Closed
mjbvz opened this issue Aug 16, 2019 · 21 comments
Closed

Help webview extensions add a Content Security Policy #79340

mjbvz opened this issue Aug 16, 2019 · 21 comments
Assignees
@mjbvz
Labels
good first issue Issues identified as good for first-time contributors help wanted Issues identified as good community contribution opportunities webview Webview issues
Milestone
On Deck

Comments

@mjbvz
Copy link
Collaborator

mjbvz commented Aug 16, 2019 •
edited
Loading

Many webview extensions do not currently set a content security policy. All webviews (even very simple ones) should set a content security policy. This is not a immediate security problem but a content security policy helps to limit the potential impact of content injections and is generally a good measure for defense in depth.

I've put together this initial list of extensions that create webviews that seem not to have a content security policy (there may be false positives). If you are feeling like a security hero, consider helping these extensions out by submitting a PR that adds a restrictive content security policy to their webviews. Here's our documentation to help you get started.

Let me know if an extension has been fixed or was incorrectly flagged

Key

  • ❗️- Confirmed and issue opened
  • ✔️ - Fixed
  • ❓ - Can't confirm in current code in github master?
  • Blank - Unconfirmed

Extensions
@mjbvz mjbvz added this to the August 2019 milestone Aug 16, 2019
@mjbvz mjbvz self-assigned this Aug 16, 2019
@mjbvz mjbvz added good first issue Issues identified as good for first-time contributors help wanted Issues identified as good community contribution opportunities webview Webview issues labels Aug 16, 2019
@ParkourKarthik
Copy link
Contributor

ParkourKarthik commented Aug 22, 2019 •
edited
Loading

I'll pick this up. Will start with code settings sync extension.

@mjbvz mjbvz modified the milestones: August 2019, September 2019 Aug 27, 2019
@utsavm9 utsavm9 mentioned this issue Aug 30, 2019
Merged
@utsavm9
Copy link
Contributor

utsavm9 commented Aug 30, 2019 •
edited
Loading

I worked on vitaliymaz.vscode-svg-previewer. It did have a CSP, which was not the most restrictive so maybe it was a false positive or maybe not.

@utsavm9
Copy link
Contributor

utsavm9 commented Aug 30, 2019 •
edited
Loading

@mjbvz I believe janisdd.vscode-edit-csv is a false positive as CSP exists in the extension. Is it here because it has unsafe-inline for its scripts and style as that is not restrictive?

@kburtram kburtram mentioned this issue Sep 5, 2019
Closed
@arjunattam arjunattam mentioned this issue Sep 5, 2019
Closed
nondanee added a commit to nondanee/vsc-netease-music that referenced this issue Sep 5, 2019
@nondanee
add CSP microsoft/vscode#79340
331e5b7
@rchiodo
Copy link
Contributor

rchiodo commented Sep 5, 2019

We've just added a csp to the python extension, but we're still getting the warning? Does it have to be strict in order to not get the warning?

Here's our current CSP:

<meta http-equiv="Content-Security-Policy" content="default-src 'unsafe-inline' 'unsafe-eval' vscode-resource: data: https: http:;">

@TantumErgo
Copy link

What file or files is the CSP usually included in?

So if you click on any of the repos with confirmed and open issues and take a look at the project files, what files should usually contain a CSP? All HTML files?

@tamuratak tamuratak mentioned this issue Sep 7, 2019
Merged
@jblievremont
Copy link

Is the declared CSP actually enforced?

We use a webview that relies upon inline CSS and base64-encoded images, and it loads correctly with a default-src 'none'; CSP.

Sure, it removes the warning. But I'm a bit worried that we'll have to push a bugfix release in a rush when our users start reporting that the webview is broken after an update to Code.

@mjbvz
Copy link
Collaborator Author

mjbvz commented Sep 13, 2019

@jblievremont Can you share a link to an example extension

@jblievremont
Copy link

@mjbvz see SonarSource/sonarlint-vscode#53

@abist
Copy link

abist commented Sep 16, 2019

Hi, I'm from the ms-mssql.mssql team. We added a CSP to our webview here. Is there anything more we need to do?

@mjbvz
Copy link
Collaborator Author

mjbvz commented Sep 16, 2019

@abist Looks good. Thanks

@tamuratak
Copy link
Contributor

✔️James-Yu.latex-workshop James-Yu/LaTeX-Workshop/pull/1676

@mjbvz mjbvz modified the milestones: September 2019, October 2019 Sep 30, 2019
@mjbvz mjbvz modified the milestones: October 2019, On Deck Oct 23, 2019
@mike-lischke
Copy link

mike-lischke.vscode-antlr4 uses webviews too and while working on adding CSPs to them I stumbled across a problem: I get the missing CSP warning even though I have added a CSP. The generated code is:

<!DOCTYPE html>
<html>
<head>
    <meta http-equiv="Content-type" content="text/html; charset=UTF-8"/>
    
            <meta http-equiv="Content-Security-Policy" content="default-src 'self';
            script-src vscode-resource: 'self' 'unsafe-inline' 'unsafe-eval' https:;
            style-src vscode-resource: 'self' 'unsafe-inline';
            img-src vscode-resource: 'self' "/>
        
            <link rel="stylesheet" type="text/css" href="vscode-resource://file///Volumes/Extern/Work/projects/vscode-antlr4/misc/light.css">
<link rel="stylesheet" type="text/css" href="vscode-resource://file///Volumes/Extern/Work/projects/vscode-antlr4/misc/dark.css">
            <base target="_blank" />
        
</head>

Why's that not accepted?

@mjbvz
Copy link
Collaborator Author

mjbvz commented Nov 13, 2019

@mike-lischke I just tried that html in a webview with VSCode 1.41.0-insider (Commit: 0d728c3) but could not reproduce the issue. Can you please share an example extension?

@mike-lischke
Copy link

@mjbvz I assumed that mike-lischke.vscode-antlr4 would give you a hint, but the current release has no CSP at all. The original code is at Github. It should run out of the box. For the effect to see you have to open an ANTLR4 grammar file. There are a few in the tests/background folder (.g4). Once you opened it right click on a rule to show the context menu in the editor pane.

Bildschirmfoto 2019-11-13 um 09 14 08

Select the Railroad diagram, which will show a webview. This one does not bring up the warning. Then try Show ATN Graph for Rule. That also opens a webview, but this time the warning mike-lischke.vscode-antlr4 created a webview without a content security policy: https://aka.ms/vscode-webview-missing-csp comes up in the debug console. This is particularly odd because both views share the same backend, with the same CSP code. Note also that the CSP code seems to be ok, since all graphics and CSS is used as expected.

@mjbvz
Copy link
Collaborator Author

mjbvz commented Nov 15, 2019

@mike-lischke I can't repo this in VS Code insiders.

Are you 100% sure your webview html always has a content security policy. For example, this line looks suspicious: https://github.com/mike-lischke/vscode-antlr4/blob/b6b065c09494e947259617661ab9f1bcb6c2a148/src/frontend/ATNGraphProvider.ts#L41

@mike-lischke
Copy link

Many thanks for looking into that @mjbvz. Unfortunately this function is not the culprit here. It definitely needs a change to include a CSP, but it is not called when you have a grammar rule selected (which is the case in this example).

I tried to debug further to see where the missing csp message is generated, but that went to nowhere (I found the place where it is printed, but not the location where vscode determines if this message is needed). My (vague) assumption is that something in the generated HTML code is keeping vscode to think the csp is still missing (even though it is there).

@apupier apupier mentioned this issue Nov 21, 2019
Merged
@zoellner zoellner mentioned this issue Feb 10, 2020
Open
@seongwon-kang seongwon-kang mentioned this issue Feb 19, 2020
Closed
@seongwon-kang
Copy link

seongwon-kang commented Feb 19, 2020 •
edited
Loading

@mjbvz Orta.vscode-jest is a test tool. and they uses webview to see html snapshot.
they escapes all html tag, and encloses with <pre> tag. it seems false positive, or may add csp tag dynamically?

@mjbvz
Copy link
Collaborator Author

mjbvz commented Feb 26, 2020

@seongwon-kang Can you link the the code that does this? All html used in webviews (even html that properly escapes its content) should have a content security policy

@seongwon-kang
Copy link

seongwon-kang commented Feb 26, 2020 •
edited
Loading

@mjbvz this will add csp tag dynamically, but I doubt this csp is correctly configured.

@seongwon-kang
Copy link

image

@mjbvz it seems vscode already attach CSP metatag when webview renders to workbench. is any other path to see webview?

@mjbvz
Copy link
Collaborator Author

mjbvz commented Oct 3, 2020

Closing this since we've done work on the VS Code side to make these errors more obvious to extension authors.

@mjbvz mjbvz closed this as completed Oct 3, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Dec 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@mjbvz mjbvz

Labels
good first issue Issues identified as good for first-time contributors help wanted Issues identified as good community contribution opportunities webview Webview issues
Projects
None yet
Milestone
On Deck
Development

No branches or pull requests
10 participants
@jblievremont @mike-lischke @abist @ParkourKarthik @TantumErgo @tamuratak @mjbvz @utsavm9 @rchiodo @seongwon-kang