All tests were doing use up-to-date browsers (Firefox 58 and Chromium 62) and the current release of Riot-Web (v0.13.5).
For postMessages, the MDN web docs write:
If you do expect to receive messages from other sites, always verify the sender's identity using the origin and possibly source properties. Any window (including, for example, http://evil.example.com) can send a message to any other window, and you have no guarantees that an unknown sender will not send malicious messages. Having verified identity, however, you still should always verify the syntax of the received message. Otherwise, a security hole in the site you trusted to send only trusted messages could then open a cross-site scripting hole in your site.
In your code for rendering the download button in your usercontent API looks like this:
window.addEventListener("message", function(e){eval("("+e.data.code+")")(e)})This does not verify anything and directly executes the code. As such, any website (iframe or so) can let it execute code. That is an avoidable risk.
Generally the JS execution can only happen in the iframe, which limits it's risk. However, the iframe contains sensitive content like the e2e encrypted image, so when one can access it.
It is rather theoretical, as – when properly setup(!) – the different origins of the iframes used in Riot (integration, usercontent) seem to prevent that one can navigate to the main page and send that message. However, it is very bad practice and may introduce vulnerabilities when there will be further iframes on your site. And even cross-site iframes are not 100%ly sandboxed out of the box. See the third attack for a demonstration of what you can still do with cross-site iframes.
Check the origin of the message when receiving it, as the MDN web docs, recommend.
In your SDK you also use postMessage to send messages between the integration server and the Riot instance.
Fortunately, when receiving the message you check the origin there. You do use this code:
const url = SdkConfig.get().integrations_ui_url;
if (event.origin.length === 0 || !url.startsWith(event.origin) || !event.data.action) {
return; // don't log this - debugging APIs like to spam postMessage which floods the log otherwise
}So let's look at the core here: url.startsWith(event.origin). So you want to verify that the (potentially malicious) sende (event.origin) is the valid one configured in the settings.
So with the default settings, this check is basically:
"https://scalar.vector.im/".startsWith("https://scalar.vector.im");
-> trueSo far, that is great. However, this check can also be bypassed. Because startsWith checks that the settings string is the beginning of the first. Well... there are many cases, where the origin may be the beginning of the string, but not a valid string.
So let's say, I can register a domain scalar.ve (.ve is the TLD of Venezuela). When I then sent a message, the check succeeds anyway:
"https://scalar.vector.im/".startsWith("https://scalar.ve");
-> trueThe catch: This is only theoretical, because actually you are lucky. The registration for the .ve TLD is restricted to the third level.
However, we are talking about a self-hosted system here. So server admins can choose any domain for their integration server and if they self-host instances and do not have "luck" with their domain, and the domain is e.g. at https://scalar.cheaphost.com, we can attack it with a domain scalar.ch:
"https://scalar.cheaphost.com/".startsWith("https://scalar.ch");
->
So the attacker at https://scalar.ch can "forge" messages so they look they may come from https://scalar.cheaphost.com/.
I have not looked at the impact for this to deeply, so it remains theoretical.
The correct check is, to just turn it around. That check does not work:
"https://scalar.ve".startsWith("https://scalar.vector.im/");
-> falseSo check wether the dubious data begins with the known safe string, not the reverse you currently do.
So the things before were theoretical. Let's get practical. For this attack we exploit the iframe usercontent and demonstrate that the sandboxing done currently, is not enough.
You use iframes for sandboxing. You load them from a third-party website and assume this makes them secure. I am very sorry to disappoint you, but it does not secure them any against any attacks. E.g. phishing.
So first a proof of concept.
Videos:
- Firefox (a hard to notice variant): https://vimeo.com/256119545
- Chrome/ium: https://vimeo.com/256119507 (the not trusted is only displayed, because of my fake domain)
- Firefox: https://vimeo.com/256119550
The password for all videos is riotIFrameAttack777.
I've attached the source in the attackIframePOC.zip. Host the fake login screen with a usual riot instance (just replace the index.html) and the attackRedirect.html as the usercontent iframe at another. Configure the third Riot instance for login just as usual, but link to attackRedirect.html for the usercontent iframe. The attackRedirect.html is also live hosted on https://rugk.neocities.org/riot/bad.html. (As you can see, that's what I've used in the videos.)
The problem is iframes (even when they are cross-origin) can still redirect the origin/top frame, i.e. the main riot site. This is then usable from an attacker to redirect a user to a phishing site. In the example this is made especially difficult to notice for the user, as we only redirect when the user is doing something else
As you can see in the videos, one can easily phish all the login credentials. One cannot break the e2e crypto, however, as the phishing site is a different domain and has thus no access to the main site.
Also, the user could be fooled in a better way when we may display the real notification, which sometimes appears (at least when you remove the device remotely), on the phishing site. See manualStop.png. That would make it even more realistic.
As long as you don't manually set allow-top-navigation this prevents this attack.
Also, BTW, sandboxed iframes also have a different same origin policy. Thus, you can – as a side effect – make selfhosting of Riot easier by just using sandboxed iframes. To demonstrate that it is possible for your use case, I have attached another "POC", see "secureSandbox.zip." Just as Riot Web, it displays a link with a blob object URI to download the image. However, it does not need yet another domain. And, what is the most important fact, it is immune against this redirect attack presented here.
Note that you should do the same for the integrations iframe, of course. As many "self-hosters" may not change it and thus depend on it for security, it is not acceptable that such an attack is possible.
Please note that at the earliest at 2018-05-18 (90 days from now) or when you've released the fix, I'll disclose these vulnerabilities.
PS: And a final word not belonging to this report, but being an important thing for a FLOSS community project: Please don't lie to your users/contributors. You should know your own source code. Where did we do that, you may ask? Well... you've repeatedly claimed that you use the iframe for PDF, image/video preview, but your source does say something else. You actually only use it for displaying the download button. To recheck, I've tested it in Riot and it does indeed use data URIs for everything else. (including all previews) So, I know, I sometimes annoyed you, but this is something you need to handle and not reject all potential contributors.
2018-04-23: Looked into issue
I'm trying to understand your PoC exploit code: in this case the malicious code is hosted by the user content renderer which isn't specifically something we're trying to protect against: we assume the user content renderer is trusted at least to the extent that the page itself is not malicious.
Is the assumption here that malicious code could be injected via the postMessage due to the lack of origin checking you point out in your first writeup? I've been looking at how this could be done in practice and it requires getting a reference to the 'window' object to post the message to. Since the usercontent frame doesn't actually display the content directly, I can't see a way to do this in practice.
We looked at using sandboxed iframes at the time, but the problem was the limitations on what could be done with the resulting blob: URLs. Your PoC fix with sandboxed iframes works for me on Firefox and Chrome 68 but not Chrome stable (65). I believe this is the reason we didn't go with sandboxed iframes when writing this. You say you used Chrome 62 to test - does it still work for you in 65?
If we can find a way to get sandboxed iframes working in the stable versions of the main browsers I'd love to switch over to using it as I agree it's better in many ways: otherwise, we may have to wait until Chrome 66/67/68 is stable.
2020-02-07: Fix provided in element-hq/element-web#12292 2020-02-10: Publicly disclosed report.
Providing the missing
manualStop.png: