Bug 575924 (CVE-2021-41038) - XSS in @theia/plugin-ext webview
Summary: XSS in @theia/plugin-ext webview
Status: RESOLVED FIXED
Alias: CVE-2021-41038
Product: Community
Classification: Eclipse Foundation
Component: Vulnerability Reports (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows 10
: P3 major (vote)
Target Milestone: ---   Edit
Assignee: Security vulnerabilitied reported against Eclipse projects CLA
QA Contact:
URL:
Whiteboard:
Keywords: security
Depends on:
Blocks:
 
Reported: 2021-09-11 18:49 EDT by NDevTK NDevTK CLA
Modified: 2022-01-14 15:26 EST (History)
7 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description NDevTK NDevTK CLA 2021-09-11 18:49:51 EDT
A webview message event has no origin or parent check as such its possible to inject HTML and JS from any opener such by using window.open()

Despite it using the sandbox attribute it has "allow-scripts allow-same-origin" making it insufficient since it will NOT get run in its own origin as intended.
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/iframe#attr-sandbox

Reason for Severity:
There is a "popular" service which uses this webview feature on the same origin hence at least for that service its possible to do code execution,
They have requested me to make this report.

POC:
w = open("https://SERVER/webview/index.html?id=a");
w.postMessage({channel: "content", args: {options: {allowScripts: true}, contents: "<script>document.write(document.domain)</script>"}}, "*");

Source of the XSS:

Gets message with no origin or parent check.
https://github.com/eclipse-theia/theia/blob/d3501165bb4e87c3612a1a02c34a1d16ab81802c/packages/plugin-ext/src/main/browser/webview/pre/host.js#L28

Content gets added to the HTML.
https://github.com/eclipse-theia/theia/blob/d3501165bb4e87c3612a1a02c34a1d16ab81802c/packages/plugin-ext/src/main/browser/webview/pre/main.js#L443

Because of the allowScripts the sandbox gets set to "allow-scripts allow-forms allow-same-origin" this serves no security benefit over not having the attribute.
https://github.com/eclipse-theia/theia/blob/d3501165bb4e87c3612a1a02c34a1d16ab81802c/packages/plugin-ext/src/main/browser/webview/pre/main.js#L480
Comment 1 Wayne Beaton CLA 2021-09-13 17:04:31 EDT
I've added the project leads in copy and have marked the issue as "committers-only" (confidential).

Our process for handing vulnerabilities is captured in the handbook.

https://www.eclipse.org/projects/handbook/#vulnerability
Comment 2 NDevTK NDevTK CLA 2021-09-14 11:38:20 EDT
Does this automatically add "committers-only" when using https://www.eclipse.org/security/

Since its a static file and window.parent.postMessage({ }, '*'); gets used anyway,
Maybe add: if (e.source !== window.parent) return
To the event listener as this would restrict messages to its embedder,
Then websites that have embed/clickjacking protection would be safe.

Origin check would be better if it was not static.

I think it has "allow-same-origin" so that .write() works but as it says srcdoc should be able to insert content anyway.

This line will forward on messages overriding the origin and adding the "bid"
So without framing protections checking the message origin would not work.
https://github.com/eclipse-theia/theia/blob/8df74a70d59136d15802ac6c55bdad0d9dc1b5f5/packages/plugin-ext/src/main/browser/webview/pre/host.js#L31
Comment 3 Wayne Beaton CLA 2021-09-15 11:08:31 EDT
(In reply to NDevTK NDevTK from comment #2)
> Does this automatically add "committers-only" when using
> https://www.eclipse.org/security/

It is supposed to, but apparently did not. I'm investigating.
Comment 4 Paul Maréchal CLA 2021-09-15 12:13:20 EDT
> There is a "popular" service which uses this webview feature on the same origin hence at least for that service its possible to do code execution,
They have requested me to make this report.

I feel like the onus is on this "popular" service to follow the Theia framework recommendations and serve their webviews on a different origin.

See https://github.com/eclipse-theia/theia/tree/master/packages/plugin-ext

Is there still an issue when the webviews are served from a different origin?

If serving from a different origin prevents the current issue, then this is the recommend way of setting up the application. AFAIK there is no ongoing work to support serving webviews from the same origin safely.
Comment 5 NDevTK NDevTK CLA 2021-09-15 13:18:41 EDT
Yes, this issue affects all webviews even when served on a different origin
Since you can access iframes of cross origin websites like w[0].postMessage()
This allows you to inject content on to any webview for any website.

The sandbox complaint means that they can use the same origin as the IDE which makes impact worse. 

"I feel like the onus is on this "popular" service to follow the Theia framework recommendations and serve their webviews on a different origin."

I have asked this already and they "didn't know about the comment about the webview"
Comment 6 Paul Maréchal CLA 2021-09-15 16:28:03 EDT
(In reply to NDevTK NDevTK from comment #5)

I wasn't able to figure out how to use your POC to reproduce the exploit, could you provide detailed instructions for it?
Comment 7 NDevTK NDevTK CLA 2021-09-15 16:48:53 EDT
w = open("https://some target.gitpod.io/");
If not already open the "Welcome" tab

w[1].postMessage({channel: "content", args: {options: {allowScripts: true}, contents: "<script>document.write(document.domain)</script>"}}, "*");

Content should get injected on the left.
Comment 8 NDevTK NDevTK CLA 2021-09-15 16:49:48 EDT
I mean right.
Comment 9 NDevTK NDevTK CLA 2021-09-15 21:21:52 EDT
I did a bad example on the welcome page you need to click on one of the "walkthroughs" then the webview should be at w[1] or w[2] you can also get a webview by clicking to view a extension that seems you a shared domain of extensioneditor-webview-foreign.ws-eu16.gitpod.io

I should note: gitpod.io was not the target its just an example.
Comment 10 NDevTK NDevTK CLA 2021-09-16 16:51:41 EDT
Do you think this issue will get fixed?

This allows for hijacking of any webview.
Even if no one wants to isolate them,
It would still be an improvement for the websites that use X-Frame-Options to have  a window.parent check on messages.
I thought this was fine since window.parent.postMessage is already used in the code.
Comment 11 Paul Maréchal CLA 2021-09-16 19:12:13 EDT
> Do you think this issue will get fixed?

For sure, just needed to put time aside to seriously look at this.

Thanks for the extra information, it finally clicked how to use your POC!

I was indeed able to reproduce.

For the fix I ended up checking if messages came through local frames, hopefully this should prevent all critical vectors?

Please tell me if you still find a vulnerability using the following patch: https://github.com/eclipse-theia/theia/pull/10125

If this is enough in your opinion then we shall merge it ASAP.
Comment 12 Paul Maréchal CLA 2021-09-16 19:14:35 EDT
Forget about the PR link, see this branch directly: https://github.com/eclipse-theia/theia/tree/mp/webview-frame-check

I thought I could close the PR and re-open when ready but GitHub wouldn't let me... TIL.
Comment 13 NDevTK NDevTK CLA 2021-09-16 19:48:01 EDT
Looks good, I think window.parent is the same as window when theirs no parent.
Not sure what use the sourceIsChildFrame exception has guess its for compatibility.
Annoyingly this still affects webviews without embed protection.
Comment 14 Marc Dumais CLA 2021-09-17 10:18:45 EDT
@NDevTK, 

Thanks for finding and reporting this vulnerability, and for the help validating Paul's potential fix.

While Paul works on the fix, could you give us a hand to come-up with a good, short description of the vulnerability, that we can use in the CVE?

Also, do you think the following CWE correctly describes the issue? 
CWE-942- "Permissive Cross-domain Policy with Untrusted Domains" ( https://cwe.mitre.org/data/definitions/942.html )

We have a planned Theia release (v1.18.0) in a couple of weeks, on September 30th, that I hope can be the version where we fix this.
Comment 15 Paul Maréchal CLA 2021-09-17 10:29:39 EDT
(In reply to NDevTK NDevTK from comment #13)
> Looks good, I think window.parent is the same as window when theirs no
> parent.

What an odd behavior, but you are right.

> Not sure what use the sourceIsChildFrame exception has guess its for
> compatibility.

IIUC the way webviews are setup we have: Theia main context > "host" iframe > webview iframe.

The code I changed lives in its own iframe and acts as a bridge between Theia and the webview frame.

> Annoyingly this still affects webviews without embed protection.

Sorry I don't know what this means and how we should guard against this?
Comment 16 NDevTK NDevTK CLA 2021-09-17 11:51:50 EDT
(In reply to Paul Maréchal from comment #15)
> (In reply to NDevTK NDevTK from comment #13)
> > Looks good, I think window.parent is the same as window when theirs no
> > parent.
> 
> What an odd behavior, but you are right.
> 
> > Not sure what use the sourceIsChildFrame exception has guess its for
> > compatibility.
> 
> IIUC the way webviews are setup we have: Theia main context > "host" iframe
> > webview iframe.
> 
> The code I changed lives in its own iframe and acts as a bridge between
> Theia and the webview frame.
> 
> > Annoyingly this still affects webviews without embed protection.
> 
> Sorry I don't know what this means and how we should guard against this?

If theirs no embed protection and the webview location is known in place like X-Frame-Options an attacker
can create there own iframe to get a XSS for that origin then the same origin policy will allow changing any webview using that origin.

The only way I know of to prevent is for the content to be isolated.
Comment 17 NDevTK NDevTK CLA 2021-09-17 11:59:43 EDT
(In reply to Marc Dumais from comment #14)
> @NDevTK, 
> 
> Thanks for finding and reporting this vulnerability, and for the help
> validating Paul's potential fix.
> 
> While Paul works on the fix, could you give us a hand to come-up with a
> good, short description of the vulnerability, that we can use in the CVE?
> 
> Also, do you think the following CWE correctly describes the issue? 
> CWE-942- "Permissive Cross-domain Policy with Untrusted Domains" (
> https://cwe.mitre.org/data/definitions/942.html )
> 
> We have a planned Theia release (v1.18.0) in a couple of weeks, on September
> 30th, that I hope can be the version where we fix this.

Description: Webview contents can be hijacked via postMessage()

"Permissive Cross-domain Policy with Untrusted Domains" looks correct but the description of "A cross-domain policy file ("crossdomain.xml" in Flash and "clientaccesspolicy.xml" in Silverlight) defines a list of domains from which a server is allowed to make cross-domain requests" seems unrelated to JavaScript.
Comment 18 Paul Maréchal CLA 2021-09-17 13:05:46 EDT
(In reply to NDevTK NDevTK from comment #16)
> If theirs no embed protection and the webview location is known in place
> like X-Frame-Options an attacker
> can create there own iframe to get a XSS for that origin then the same
> origin policy will allow changing any webview using that origin.
I don't understand how this attack would work... If you are able to give clear instructions on how to reproduce such an exploit it would help tremendously.

In the first exploit, the attacker could have been any website/origin distributing the code from your POC which would open a "hijack-able" Theia instance.

Who/where is the attacker in this second scenario? Right now it feels like the attacker would be another webview? Or is it still some other website/origin?

> The only way I know of to prevent is for the content to be isolated.
Does that mean being served on its own (somewhat random) origin?

With the fix on my current branch, only messages coming from the parent of the webview will go through. The webview then has to send messages to the parent (Theia) to do any meaningful work.

If someone were to try and open just a webview in an iframe from some other website/origin they'd be missing the important Theia bits, so I have difficulties understanding what's left to attack?
Comment 19 Marc Dumais CLA 2021-09-17 14:17:23 EDT
(In reply to NDevTK NDevTK from comment #17)
> 
> Description: Webview contents can be hijacked via postMessage()

Short and sweet - thanks! 

 
> "Permissive Cross-domain Policy with Untrusted Domains" looks correct but
> the description of "A cross-domain policy file ("crossdomain.xml" in Flash
> and "clientaccesspolicy.xml" in Silverlight) defines a list of domains from
> which a server is allowed to make cross-domain requests" seems unrelated to
> JavaScript.

I have a couple of alternative CWEs and might apply better. WDYT?

CWE-940: Improper Verification of Source of a Communication Channel
 ( https://cwe.mitre.org/data/definitions/940.html )

CWE-749: Exposed Dangerous Method or Function ( https://cwe.mitre.org/data/definitions/749.html)
Comment 20 NDevTK NDevTK CLA 2021-09-17 14:29:34 EDT
(In reply to Paul Maréchal from comment #18)
> (In reply to NDevTK NDevTK from comment #16)
> > If theirs no embed protection and the webview location is known in place
> > like X-Frame-Options an attacker
> > can create there own iframe to get a XSS for that origin then the same
> > origin policy will allow changing any webview using that origin.
> I don't understand how this attack would work... If you are able to give
> clear instructions on how to reproduce such an exploit it would help
> tremendously.
> 
> In the first exploit, the attacker could have been any website/origin
> distributing the code from your POC which would open a "hijack-able" Theia
> instance.
> 
> Who/where is the attacker in this second scenario? Right now it feels like
> the attacker would be another webview? Or is it still some other
> website/origin?
> 
> > The only way I know of to prevent is for the content to be isolated.
> Does that mean being served on its own (somewhat random) origin?
> 
> With the fix on my current branch, only messages coming from the parent of
> the webview will go through. The webview then has to send messages to the
> parent (Theia) to do any meaningful work.
> 
> If someone were to try and open just a webview in an iframe from some other
> website/origin they'd be missing the important Theia bits, so I have
> difficulties understanding what's left to attack?

All webviews use the same origin as long as they use the same endpoint since the sandbox attribute has "allow-same-origin" by removing that every webview would get its own origin. I was going to provide a POC for this but gitpod seems to block iframe embeds of the webview anyway :D
Comment 21 NDevTK NDevTK CLA 2021-09-17 16:30:31 EDT
(In reply to Marc Dumais from comment #19)
> (In reply to NDevTK NDevTK from comment #17)
> > 
> > Description: Webview contents can be hijacked via postMessage()
> 
> Short and sweet - thanks! 
> 
>  
> > "Permissive Cross-domain Policy with Untrusted Domains" looks correct but
> > the description of "A cross-domain policy file ("crossdomain.xml" in Flash
> > and "clientaccesspolicy.xml" in Silverlight) defines a list of domains from
> > which a server is allowed to make cross-domain requests" seems unrelated to
> > JavaScript.
> 
> I have a couple of alternative CWEs and might apply better. WDYT?
> 
> CWE-940: Improper Verification of Source of a Communication Channel
>  ( https://cwe.mitre.org/data/definitions/940.html )
> 
> CWE-749: Exposed Dangerous Method or Function (
> https://cwe.mitre.org/data/definitions/749.html)

I think its CWE-940: Improper Verification of Source of a Communication Channel
Comment 22 NDevTK NDevTK CLA 2021-09-17 20:09:08 EDT
The window.parent check restricts the messages to be from the iframe parent.

This iframe can be on attacker controlled website if the embedding policy allows it.

A postMessage can be sent to the iframe to do a XSS.

After that the same origin policy will allow accessing webviews from that origin without bypassing the window.parent check.

Looked at https://blog.mattbierner.com/vscode-webview-web-learnings/ from the README
it seems the issues with such sandbox is "The requirements around content security policy, service workers, and nested iframes"


It would be better for fixing this if the source origin was checked otherwise I guess it will just be expected for webviews to X-Frame-Options or frame-ancestors to prevent webview hijacking.
Comment 23 Paul Maréchal CLA 2021-09-20 10:51:26 EDT
I feel like someone creating a webview from a separate website/origin is not causing any vulnerability in Theia applications as the webview won't be able to access any Theia application features?
Comment 24 NDevTK NDevTK CLA 2021-09-21 05:13:11 EDT
If a Iframe can be created with a webview endpoint Theia is using,
Then it can be hijacked like:
f = document.createElement("iframe");
f.src = targetWebview;
document.body.appendChild(f);
f.contentWindow.postMessage({channel: "content", args: {options: {allowScripts: true}, contents: "<script>w = open(targetTheia);w[1].document.body.innerHTML = "hijacked"</script>"}}, "*");

This code will still have access to the bid.
Comment 25 NDevTK NDevTK CLA 2021-09-21 16:32:57 EDT
(In reply to Paul Maréchal from comment #23)
> I feel like someone creating a webview from a separate website/origin is not
> causing any vulnerability in Theia applications as the webview won't be able
> to access any Theia application features?

As long as the webview blocks crossorigin embeds and the webview is only used to display one origin and uses a different eTLD (part of the public suffix list) there are no security issues that im aware of after this patch.
Comment 26 Marc Dumais CLA 2021-09-29 10:31:46 EDT
Paul, how is this looking, for including your patch for tomorrow's release? The feedback here seems positive, but I do not understand the details or know if your patch might be risky, to be included last-minute.
Comment 27 Marc Dumais CLA 2021-09-30 11:54:10 EDT
Paul and I spoke offline. The conclusion is that we will not be able to fully address this vulnerability for todays Theia v1.18.0 release. However we do not need to wait for v1.19.0 - rather we plan to do a v1.18.1 release, as soon as we have a proper fix.
Comment 28 NDevTK NDevTK CLA 2021-09-30 20:20:52 EDT
(In reply to Marc Dumais from comment #27)
> Paul and I spoke offline. The conclusion is that we will not be able to
> fully address this vulnerability for todays Theia v1.18.0 release. However
> we do not need to wait for v1.19.0 - rather we plan to do a v1.18.1 release,
> as soon as we have a proper fix.

Any idea on how this can be fixed?
Seems like it has to be dynamic or fix the sandbox.
Comment 29 Marc Dumais CLA 2021-10-01 13:24:17 EDT
(In reply to NDevTK NDevTK from comment #28)
> (In reply to Marc Dumais from comment #27)
> > Paul and I spoke offline. The conclusion is that we will not be able to
> > fully address this vulnerability for todays Theia v1.18.0 release. However
> > we do not need to wait for v1.19.0 - rather we plan to do a v1.18.1 release,
> > as soon as we have a proper fix.
> 
> Any idea on how this can be fixed?
> Seems like it has to be dynamic or fix the sandbox.

Not yet. We have the partial fix on Paul's branch above, and Paul plans to have a deeper look soon, to search for a more complete solution. We do not know for sure that it's possible.
Comment 30 Paul Maréchal CLA 2021-10-07 15:22:34 EDT
> If a Iframe can be created with a webview endpoint Theia is using

> All webviews use the same origin as long as they use the same endpoint since the sandbox attribute has "allow-same-origin" by removing that every webview would get its own origin. I was going to provide a POC for this but gitpod seems to block iframe embeds of the webview anyway :D

If this secondary exploit can be prevented by a web server configuration then I'd argue that this is out of the hands of the framework itself.

If you confirm that you cannot trigger what you described on Gitpod, then I'll see to reach out to them to understand what we should recommend Theia application deployers to do.
Comment 31 NDevTK NDevTK CLA 2021-10-07 20:17:58 EDT
(In reply to Paul Maréchal from comment #30)
> > If a Iframe can be created with a webview endpoint Theia is using
> 
> > All webviews use the same origin as long as they use the same endpoint since the sandbox attribute has "allow-same-origin" by removing that every webview would get its own origin. I was going to provide a POC for this but gitpod seems to block iframe embeds of the webview anyway :D
> 
> If this secondary exploit can be prevented by a web server configuration
> then I'd argue that this is out of the hands of the framework itself.
> 
> If you confirm that you cannot trigger what you described on Gitpod, then
> I'll see to reach out to them to understand what we should recommend Theia
> application deployers to do.

Seem to get a http status code of 401 when I try to use a iframe on Gitpod webviews.
While they do allow embeds I think because of samesite cookies its unauthenticated so it gets blocked.
Comment 32 NDevTK NDevTK CLA 2021-10-07 20:31:45 EDT
CSP: frame-ancestors would work well to prevent embedding for the browsers it supports. X-Frame-Options would probably not work since its only SAMEORIGIN.
Comment 33 Paul Maréchal CLA 2021-10-27 14:14:41 EDT
Hi, after looking at this a bit more here's my understanding:

We merged a patch that prevents the first exploit you mentioned in that issue.

In the second exploit you described that would still work you say the attack would originate from a different origin/website. In such a case it seems this would be fixed by having a proper authentication put in front of the Theia instance to prevent other origins from interacting with it (just like Gitpod does which prevented you from executing the exploit you had described).

In general, when something can be fixed reasonably from outside of Theia applications then this is the recommended way.

Hence I would consider this issue as resolved, unless I'm still missing something.
Comment 34 NDevTK NDevTK CLA 2021-10-27 14:21:16 EDT
Seems like the parent check is enough.
As long as people are aware they need to use frame-ancestors.
SameSite cookies can also prevented it but not from the same eTLD+1.
Comment 35 Marc Dumais CLA 2021-10-28 11:29:13 EDT
(In reply to NDevTK NDevTK from comment #34)
> Seems like the parent check is enough.
> As long as people are aware they need to use frame-ancestors.

Would you mind expanding on this a little? We can add necessary documentation to help adopters/extenders do the right thing, but I feel we do not understand enough to know how to phrase this one.
Comment 36 Marc Dumais CLA 2021-10-28 11:49:37 EDT
Wayne, here's the tentative CVE - I just need Paul to confirm the version where we have the fix:

Description: In @theia/plugin-ext, Webview contents can be hijacked via postMessage(). 
Categorization: CWE-940: Improper Verification of Source of a Communication Channel
Versions: <= 1.17.2 (1.18.0 has the fix)
Comment 37 NDevTK NDevTK CLA 2021-10-28 12:35:53 EDT
(In reply to Marc Dumais from comment #35)
> (In reply to NDevTK NDevTK from comment #34)
> > Seems like the parent check is enough.
> > As long as people are aware they need to use frame-ancestors.
> 
> Would you mind expanding on this a little? We can add necessary
> documentation to help adopters/extenders do the right thing, but I feel we
> do not understand enough to know how to phrase this one.

Not sure maybe something like:
Webviews will trust and run code from there embedder so only allow embedding from trusted origins.
This can be done by adding a header like Content-Security-Policy: frame-ancestors 'self' https://IDE;
Comment 38 Marc Dumais CLA 2021-10-29 14:46:20 EDT
Paul has confirmed offline that the version with the fix is indeed 1.18.0. I think we are done, from the project's side. 

Wayne, let us know if you need anything else from us.
Comment 39 Wayne Beaton CLA 2021-11-10 12:15:15 EST
We'll use CVE-2021-41038.
Comment 40 NDevTK NDevTK CLA 2021-12-28 21:29:47 EST
To clarify the "popular" service I was referring to is Google Cloud Shell,
They use this patch with CSP frame-ancestors 'self' to prevent XSS.

My bad write up: https://docs.google.com/document/d/1cvWRwUSQCAmTy2bQRLeFp6EObnmi9gGZW4sxbjQPTAg/edit
Comment 41 Marc Dumais CLA 2022-01-14 15:26:39 EST
Nice, thanks for sharing