Replace cross origin renderer (usercontent.riot.im) with something better #6173

rugk · 2018-02-15T19:30:09Z

For what is usercontent.riot.im used? I just saw it my instance tried to make a connection to it, but why?

Actually it should be self-hosted and save images or so to Synapse, so what does it do?

I saw it only in encrypted group chats.

STR:

In an encrypted chat, upload a file.
Click on "Encrypt file".
-> Instead of Synpase, usercontent.riot.im is contacted.

The text was updated successfully, but these errors were encountered:

ara4n · 2018-02-15T19:41:16Z

it's a wrapper used to host and display content decrypted from e2e in a dedicated domain to prevent XSS attacks - after all, you don't want someone to send you some HTML+JS over e2e, which you then decrypt in JS in your browser and end up executing when you display it to the user.

You could also host your own, as long as it's on a separate domain to your riot-web. in general people don't bother, given it's an entirely trivial static system (a bit like matrix.to), although you might want to if you want to have full control.

rugk · 2018-02-15T19:42:06Z

The site https://usercontent.riot.im/v1.html just returns this code:

<html>
<head>
<script>
window.addEventListener("message", function(e){eval("("+e.data.code+")")(e)})
</script>
</head>
<body></body>
</html>

This is then injected into the current site as an (of course, not sandboxed!) iframe in order to show the content in it!
Why not just create that HTML code and iframe via JavaScript? Why do you have to access yet another of your own domains, outside of the admin's control?

And why do you always do so in encrypted chats? This whole thing makes no sense…

That is not only basically a backdoor for all self-hosted instances, because in order to get any downloaded image in an e2e chat it contacts a site of yours, which could just serve any JavaScript. Again, the iframe is not sandboxed, so it when the JS is malicious it may do bad things. At the very least you could easily intercept all attachments. To repeat: You could intercept e2e encrypted images as you use a centralized domain! WTF…

And again your breaking the whole decentralization here. I thought you are making a decentralized product and not a "we decentralize it, but hide some nefarious requests to centralized domains, so we can still take it over if we want" product.

t3chguy · 2018-02-15T19:48:15Z

Its right there in the readme, you are able to host your own

rugk · 2018-02-15T19:48:48Z

you don't want someone to send you some HTML+JS over e2e, which you then decrypt in JS in your browser and end up executing when you display it to the user.

Just don't display HTML/JS files and images and stuff can be displayed in img tag where they can do nothing.
Also the idea may not be bad (I am all for sandboxing!), but you have to do it in the correct way:

Do not load third-party content!
Create an iframe via JS (or have it in the index.html).
And finally sandbox it.

BTW currently thanks to the missing sandbox I would not say this protection even works. If one can inject JS then okay, then one can only execute JS in the iframe. That still means that JS can stil do things like redirect the parent(!) website and such stuff. You have to use iframes properly.

rugk · 2018-02-15T19:50:50Z

Okay, good that I can self-host it, but that is still useless. An iframe without sandbox attribute does not help much.

Also why do you only iframe e2e encrypted content, not for regular chats? Are regular chats worth less or what? There, content is directly put into a div on the site. So what?

rugk · 2018-02-15T19:57:13Z

So here is what to do:

Create an iframe via JS (or have it in the index.html), so that this extra domain is not needed.
Sandbox it.
Do the same for all untrusted stuff you load in there.

In a sandbox it cannot even do form submissions, so AFAIK it cannot do anything bad here.

Also, of course, you should not display HTML content or so inside the app anyway. (and you don't do so), so that is no big problem. It's just a link, for what's it's worth.
And images can be loaded via data: or so in an img tag. No problem. What do you even try to protect against here?

I see now why you do so. For the same reason why you recommend hosting the synapse server at a different domain. But if you properly sandbox the content and don't do obvious stupid things like displaying HTML files in the app (which you don't do currently), that is all safe!

turt2live · 2018-02-15T20:03:20Z

And images can be loaded via data: or so in an img tag

Browsers have a length limitation on this, some more strict than others. It would be a real shame to break someone's upload because they wanted to share a panoramic view of their city.

rugk · 2018-02-15T20:03:35Z

I mean you also load text into your chat application, right? So you hopefully properly sanitize that, yes? So why don't you put that into a untrusted domain name and iframe it?
That could be made, it's just kinda ridiculous.

It's the same for these images. If you just put a base64-encoded blob into the site in an img tag, what could be done? Nothing…

So maybe sandbox that media stuff, right, but then in an iframe with sandbox attribut.

ara4n · 2018-02-15T20:12:19Z

If you have found a hole in the current sandboxing, please disclose the details to security@matrix.org.

Otherwise, please read the comments in the code for an explanation of why we sandbox in this manner, and why we don't use the sandbox attribute on iframes, and why this applies to E2E content rather than non-E2E stuff.

Closing this given it's not actually a bug.

rugk · 2018-02-15T20:14:40Z

Browsers have a length limitation on this, some more strict than others. It would be a real shame to break someone's upload because they wanted to share a panoramic view of their city.

Never experienced a length problem there. And looking it up, it also seems that (also for blob URIs) very big files are supported.

rugk · 2018-02-15T22:31:21Z

Okay, so restart. Thanks for the link to the comments, that was really helpful. But I've looked into the source. And I still don't understand some things.

First, the comments:

// For attachments downloaded directly from the homeserver we can use
// Content-Security-Policy headers to disable script execution.

Note that CSP only applies when you access the file directly. I.e. the CSP, which is sent on the site you access in the browser (index.html or so) is applied. When you fetch a image.png there, it does not matter what CSP header that image sends.

But the good news is: The loading is, of course, still secure, as you just use src="https//something…" and that is plain old HTML and does never execute any JS.

Okay, I understand the Chrome limitation (or I take it as given, have not tested it) and you may use Blob URIs, fine.

// Blob URLs are generated using window.URL.createObjectURL and unforuntately
// for our purposes they inherit the origin of the page that created them.
// This means that any scripts that run when the URL is viewed will be able
// to access local storage.

createObjectURL turns bytes into a blob string. Nothing more. There is no attack vector here, so what do you fear could happen?
See below for where I see a potential attack.

// the downside of using a sandboxed iframe is that the browers are overly
// restrictive in what you are allowed to do with the generated URL.

You get a PR; where I apply a sandbox. It's not "overly restrictive". You can easily specify what should be allowed to in the sandbox atttribut. And we need allow-scripts here, of course. (toplevel access should be forbitten, however, by doing so)

I've also had a look on the Telegram and WhatsApp attacks again. As you can see the problem occurs when you access blob: or filesystem: URIs in a new tab, as only then they can get to the HTML root and create a fake HTML file.
So that is the only case where createObjectURL could cause problems.

So I tried to open it in a new tab via right-click and it did not work. Wow, great! Good job! I have no idea how you did it, but it does not work.

But there is one thing, I've also noticed. That whole cross-origin/sandbox thing only applies to unknown file types. Images and videos, e.g., are included as data URIs.

SO:

It would be a real shame to break someone's upload because they wanted to share a panoramic view of their city.

Yes, and it would still do in your case, as you use data URIs for images. Only the download button for any file type uses this iframe.

So finally some questions:

How could you prevent it from opening in a new tab? I could not
Do you somehow handle the click on the download button? I just could not click on it when duplicating the element and moving it out of the iframe, so, I just wonder here.

albjeremias · 2018-03-07T14:26:09Z

Please re-open this issue for clarification! Thanks.

ara4n · 2018-03-07T14:40:57Z

@rugk kindly provided a proof of concept for how we might be able to get rid of usercontent.riot.im in favour of a dynamically generated iframe with appropriate iframe sandboxing; we are hoping to dig into it & test/understand it over the coming month. When we tried this 1-2 years ago we couldn't get it to work due to overzealous sandboxing support in browsers, but it's quite possible that we missed something. This would have the advantage of even better sandboxing the content (assuming that you are on a browser which fully implements iframe sandboxing)

rugk · 2018-03-07T14:42:16Z

Not needed, I know they are doing something behind the scenes. At least that's what they told.

ara4n · 2018-03-07T14:48:17Z

i assume our messages crossed, but yup: we're doing something behind the scenes (i.e. analysing @rugk's findings)

ara4n · 2018-04-02T09:46:17Z

(just checking in to confirm that this is still on our radar).

dbkr · 2018-04-24T16:34:18Z

I've just been looking at the concerns here. Let me start by clarifying the purpose of the cross origin renderer and elaborate a little on what Matthew said above. Having an iframe with a different origin allows us to set the origin of the blob URI we create to an origin other than whatever you host riot on, which means any HTML/JS it might execute is in a separate origin and so can't access your private keys.

As for sandboxing, the reason the iframe isn't sandboxed is that it doesn't work with a sandboxed iframe in the current stable version of Chrome (65): the created blob URI can't then be downloaded.

There were also two specific issues @rugk raised that are related to this cross-origin renderer. The first was lack of checking on the origin of messages in the cross-origin renderer code meaning that, in theory, an attacker could post a message to the cross-origin iframe and execute code. I'm not aware of any way this could actually happen since the attacker would need a window object to post a message to it. Nonetheless, it makes sense to have an extra layer of security here so I've made matrix-org/matrix-react-sdk#1849 / matrix-org/usercontent#1. Anyone who would like this extra layer of security can update their cross-origin renderer to this code at any time: it's backwards compatible in both directions.

The second was a proof-of-concept phishing attack whereby the cross-origin iframe changes the location of its parent, redirecting the user's browser to a malicious site when the tab is backgrounded. It is reliant upon getting malicious code into the cross origin iframe, but given, as above, there is no way for a site to get a window object for the iframe, we don't believe this is possible. The origin lock further protects against this.

It’s true that the host of the cross-origin renderer (ie. usercontent.riot.im by default) could inject malicious code: this isn't something we've tried to protect against although we agree it’s pretty undesirable. If this is a concern, please host your own cross-origin renderer as per the cross_origin_renderer_url config parameter at https://github.com/vector-im/riot-web#configjson

The good news is that Chrome 68 now does allow you to download blob URLs created in sandboxed iframes, so once this is generally available we will be very happy to move to sandboxed iframes and we'll no longer need the cross origin renderer mess.

Many thanks to @rugk for digging into this and responsibly disclosing the various possible attack vectors here and encouraging us to move over to sandboxed iframes as soon as we can.

albjeremias · 2018-04-24T16:52:07Z

@dbkr my concern about this issue is about the user being tracked whenever it makes a new post with attachment to riot... how many hacks a sysadmin needs to do, in order to setup his own usercontent host? If this only affects riot-web how does riot-android and other matrix clients exchange attachments?

from what I understand (and I have to be clear that I don't understand all of the arguments you are putting here), this does not look like a good approach, but just a hack so that we can have (encrypted) attachments for now in riot!

ara4n · 2018-04-24T17:18:36Z

it's trivial to set up your own usercontent host, and it's is a valid solution to securing E2E attachments. good news is that once Chrome 68 is mainstream we should be able to switch over to using sandboxed iframes and remove the usercontent host entirely.

dbkr · 2018-04-24T19:51:28Z

@albjeremias riot-android isn't a web client and so doesn't have to worry about script origins, etc. You're correct though, it's far from the ideal approach, but right now it's the only way to do e2e attachments securely at all.

rugk · 2018-04-25T10:36:45Z

The origin lock further protects against this.

It does not matter here, but that's not true. Browsers allow redirecting the parent frame site even though the origin is different. That's a crazy "feature", I know, but it's true. This is the whole thing that phishing attack was based on.

Anyway I'll publish the full report soon.

stoically · 2020-02-06T09:36:36Z

Random idea: What if the homeserver (synapse) serves the v1.html? That would remove the usercontent.riot.im requirement and still allow the additional safety of a different origin to do unsafe-eval things. Though, I'm not sure whether it's a good idea to operate with the homeservers origin.

t3chguy · 2020-02-06T09:39:42Z

nowadays it should be doable without such an external resource using a sandboxed iframe

stoically · 2020-02-06T09:43:35Z

The challenge with data: iframes is that they inherit the parent document CSP - so the origin hosting riot would still need an unsafe-eval CSP in order for the iframe to work.

t3chguy · 2020-02-06T09:46:16Z

the eval is only needed to make use of the generic usercontent sandbox via postmessage, if we can use a data iframe we should be able to do without eval

rugk · 2020-02-10T16:23:05Z

Great there is a fix now. FYI as promised before but long forgotten, here is the full report disclosure (I just added the dates at the end, everything else is the original report from

https://gist.github.com/rugk/2e5caef4a266f15d71eae1b53ff8c29a

Summary: Update Riot to 1.5.10 **Changes:** - Get rid of dependence on usercontent.riot.im (for details see [here](element-hq/element-web#6173)) Test Plan: Sent a few messages, stickers and files, made a short conference call Reviewers: #triage_team, JoshStrobl Reviewed By: #triage_team, JoshStrobl Subscribers: JoshStrobl Differential Revision: https://dev.getsol.us/D8306

ara4n closed this as completed Feb 15, 2018

ara4n reopened this Mar 7, 2018

This was referenced Apr 2, 2018

Simplify the CSS generated by computedStyle for iframe'd download links #6418

Closed

Replace readBlobAsDataUri with URL.createObjectURL matrix-org/matrix-react-sdk#1820

Closed

lampholder added the Security label Apr 16, 2018

dbkr changed the title ~~usercontent.riot.im ??~~ Replace cross origin renderer (usercontent.riot.im) with something better Apr 24, 2018

dbkr removed the Security label Apr 24, 2018

lampholder added T-Defect P2 labels Apr 26, 2018

lampholder added the S-Minor Impairs non-critical functionality or suitable workarounds exist label Apr 26, 2018

ara4n mentioned this issue Apr 30, 2018

Switch to using sandboxed iframes for displaying attachments #6639

Closed

ara4n added T-Task Tasks for the team like planning Security and removed T-Defect S-Minor Impairs non-critical functionality or suitable workarounds exist labels Jun 4, 2018

albjeremias mentioned this issue Jun 21, 2018

E2EE: Image from clipboard data / upload is sent unencrypted mujx/nheko#348

Open

ara4n mentioned this issue Jun 1, 2019

Clicking "Decrypt" on a file turns the filesize numbers into emoji #9867

Closed

uhoreg added the A-E2EE label Jan 15, 2020

This comment has been minimized.

Sign in to view

t3chguy self-assigned this Feb 6, 2020

t3chguy mentioned this issue Feb 7, 2020

Get rid of dependence on usercontent.riot.im #12292

Merged

t3chguy added this to In Progress in Web App Team via automation Feb 7, 2020

t3chguy moved this from In Progress to In Review in Web App Team Feb 13, 2020

t3chguy closed this as completed in #12292 Feb 19, 2020

Web App Team automation moved this from In Review to In Test Feb 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace cross origin renderer (usercontent.riot.im) with something better #6173

Replace cross origin renderer (usercontent.riot.im) with something better #6173

rugk commented Feb 15, 2018

ara4n commented Feb 15, 2018

rugk commented Feb 15, 2018

t3chguy commented Feb 15, 2018

rugk commented Feb 15, 2018 •

edited

Loading

rugk commented Feb 15, 2018

rugk commented Feb 15, 2018 •

edited

Loading

turt2live commented Feb 15, 2018

rugk commented Feb 15, 2018

ara4n commented Feb 15, 2018

rugk commented Feb 15, 2018 •

edited

Loading

rugk commented Feb 15, 2018

albjeremias commented Mar 7, 2018

ara4n commented Mar 7, 2018

rugk commented Mar 7, 2018

ara4n commented Mar 7, 2018

ara4n commented Apr 2, 2018

dbkr commented Apr 24, 2018

albjeremias commented Apr 24, 2018

ara4n commented Apr 24, 2018

dbkr commented Apr 24, 2018

rugk commented Apr 25, 2018

stoically commented Feb 6, 2020 •

edited

Loading

t3chguy commented Feb 6, 2020

stoically commented Feb 6, 2020

t3chguy commented Feb 6, 2020

This comment has been minimized.

rugk commented Feb 10, 2020

Replace cross origin renderer (usercontent.riot.im) with something better #6173

Replace cross origin renderer (usercontent.riot.im) with something better #6173

Comments

rugk commented Feb 15, 2018

ara4n commented Feb 15, 2018

rugk commented Feb 15, 2018

t3chguy commented Feb 15, 2018

rugk commented Feb 15, 2018 • edited Loading

rugk commented Feb 15, 2018

rugk commented Feb 15, 2018 • edited Loading

turt2live commented Feb 15, 2018

rugk commented Feb 15, 2018

ara4n commented Feb 15, 2018

rugk commented Feb 15, 2018 • edited Loading

rugk commented Feb 15, 2018

albjeremias commented Mar 7, 2018

ara4n commented Mar 7, 2018

rugk commented Mar 7, 2018

ara4n commented Mar 7, 2018

ara4n commented Apr 2, 2018

dbkr commented Apr 24, 2018

albjeremias commented Apr 24, 2018

ara4n commented Apr 24, 2018

dbkr commented Apr 24, 2018

rugk commented Apr 25, 2018

stoically commented Feb 6, 2020 • edited Loading

t3chguy commented Feb 6, 2020

stoically commented Feb 6, 2020

t3chguy commented Feb 6, 2020

This comment has been minimized.

rugk commented Feb 10, 2020

rugk commented Feb 15, 2018 •

edited

Loading

rugk commented Feb 15, 2018 •

edited

Loading

rugk commented Feb 15, 2018 •

edited

Loading

stoically commented Feb 6, 2020 •

edited

Loading