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.

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

IFrame can access mashlibs window causing XSS #372

Open
Otto-AA opened this issue Mar 16, 2023 · 8 comments
Open

IFrame can access mashlibs window causing XSS #372

Otto-AA opened this issue Mar 16, 2023 · 8 comments

Comments

@Otto-AA
Copy link

Otto-AA commented Mar 16, 2023

Summary

When a user opens a html file with mashlib, this file can access the window of mashlib and thus among other things the authenticated fetch at UI.authn.session.fetch.

Reproducing

Steps to reproduce (on NSS):

  1. open yourpod.solidcommunity.net
  2. login (required precondition for the exploit to work)
  3. create a file test.html at any location (in a real exploit the attacker with append/write permissions would create this)
  4. Write the html below into the file. Change the url with a private file
  5. Display the html with mashlib (click on the "d" symbol)
  6. Observe one alert with your webid and one with the file contents
<script>
  const main = async () => {
    mashlibWindow = window.parent;
    alert('Logged in as ' + await mashlibWindow.UI.authn.checkUser())
    res = await mashlibWindow.UI.authn.session.fetch('https://yourpod.solidcommunity.net/private/secret.txt')
    alert('Secret: ' + await res.text())
  }
  main()
</script>

For CSS, the steps are similar, however I think the test.html file would need to be publicly readable to be loaded in the iframe. In NSS this is not necessary because it also uses cookies.

Impact

Opening a malicious html file can result in the attacker gaining control over the whole pod.

Fix

Use an iframe with the sandbox attribute which may not include allow-same-origin. See for instance this article on sandboxing: https://web.dev/sandboxed-iframes/

This would also be fixed, if pods don't serve or sandbox html files (see https://forum.solidproject.org/t/is-it-secure-for-pods-to-serve-html-files/6379/2)
EDIT: after looking at the code, I don't think it matters if pods sandbox the html, as they are not directly included with <iframe src=".../test.html"> but fetched manually and then added as a blob.
EDIT 2: it's more complicated than I thought, SolidOS has 3 different ways of creating iframes. See here for a small discussion)

@josephguillaume
Copy link
Contributor

josephguillaume commented Mar 17, 2023

Good summary of the issue, and also a good reminder why we moved to dpop rather than using cookies.

As noted, you have to be logged in and opening an untrusted HTML file on your own pod for this to work.
Automatically loading index.html files (SolidOS/solidos#196) increases the risk here, but ultimately it depends on whether you expect that an app to which you explicitly granted full access is actually trustworthy or not. If they are not, then they had full access to your pod anyway, so maybe they don't need to use this attack vector.
Once client IDs are more commonly used in ACLs, then this vulnerability would perhaps allow gaining access to the whole pod?

I don't think the dokieli or humanReadablePane panes support opening remote HTML files, but if I'm wrong and they do, then this is a much bigger issue.

Btw, I happen to be using this feature to dynamically register panes :-)
SolidOS/pane-registry#28

@josephguillaume
Copy link
Contributor

Worth noting this is already in the code:

// @@ Note below - if we set ANY sandbox, then Chrome and Safari won't display it if it is PDF.
// https://developer.mozilla.org/en-US/docs/Web/HTML/Element/iframe
// You can't have any sandbox and allow plugins.
// We could sandbox only HTML files I suppose.
// HTML5 bug: https://lists.w3.org/Archives/Public/public-html/2011Jun/0330.html
// iframe.setAttribute('sandbox', 'allow-same-origin allow-forms'); // allow-scripts ?? no documents should be static
// iframe.setAttribute('height', '480')
// iframe.setAttribute('width', '640')
const tr = myDocument.createElement('TR')
tr.appendChild(frame)
div.appendChild(tr)
return div

@Otto-AA
Copy link
Author

Otto-AA commented Mar 17, 2023

a good reminder why we moved to dpop rather than using cookies.

I've written this exploit code with NSS in mind, so this one relies on cookies to load the test.html iframe successfully (EDIT: not sure about this, as it loads the iframe differently than I've expected). A similar exploit would also work without cookies, if test.html is in a publicly readable location (ie it has stronger preconditions).

but ultimately it depends on whether you expect that an app to which you explicitly granted full access is actually trustworthy or not. If they are not, then they had full access to your pod anyway, so maybe they don't need to use this attack vector.

You could make this exploit work with the publicly appendable /inbox/ if the user opens the incoming html file with the databrowser, thus it works for an unauthenticated, completely untrusted agent. In the more general case, if I give Alice access to /foo/alice-stuff/ she doesn't have control over the full pod but could use this exploit against me. So the problem is, that you trust someone to a specific extent (Alice may append new data to folder X), but with this exploit they obtain higher permissions.

Once client IDs are more commonly used in ACLs, then this vulnerability would perhaps allow gaining access to the whole pod?

This is exactly what's happening already, but by escalating privileges given to WebIds or even unauthenticated agents. In the future, potentially also apps with client IDs.

I don't think the dokieli or humanReadablePane panes support opening remote HTML files, but if I'm wrong and they do, then this is a much bigger issue.

Not necessarily. If they embed remote html files, classic security preventions for cross-origin apply. For instance, this exploit would not work

I don't think the dokieli or humanReadablePane panes support opening remote HTML files, but if I'm wrong and they do, then this is a much bigger issue.

It would for sure be dangerous, but I'm not even sure if it's more of an issue than the current situation. Instead of opening a /inbox/ file the user would open a https://remote.com file. The impact is the same, the user interaction part seems also the same likely to happen (depending on the UI for the remote access I guess).

Btw, I happen to be using this feature to dynamically register panes :-)
SolidOS/pane-registry#28

Ay, that's a good point and reminder why discussion is needed :D

It would be possible to sandbox normal html files and load custom pane html files from a trusted (mashlib specific folder?) source. But that of course adds complexity and is time-consuming to implement.

@josephguillaume
Copy link
Contributor

You could make this exploit work with the publicly appendable /inbox/ if the user opens the incoming html file with the databrowser, thus it works for an unauthenticated, completely untrusted agent

Good point. I absolutely agree that untrusted agents should not be permitted to write HTML/executable code to a pod. This not something that solidos can solve alone, so an issue needs to be raised with NSS and CSS.

Sandboxing the IFrame would prevent an exploit that is currently likely to be possible: getting persistent access to a pod by getting someone to open their inbox while logged in + autoloading of a malicious index.html (SolidOS/solidos#196) + changing ACL permissions.
Without access to an auth token, there's not a huge amount the script could do, but disabling scripts/sanitising for the IFrame would additionally prevent access to local storage, and access to cookies? - though these should already be considered shared across resources on the pod anyway, and therefore shared between locally hosted apps.

@josephguillaume
Copy link
Contributor

josephguillaume commented Mar 18, 2023

Some additional comments on the above mention:

  • I've suggested that CSS implement a storage middleware as a server side mitigation
  • It turns out that exploiting the index.html pathway I suggested in the previous comment is surprisingly difficult with current mashlib (1.8.8)+CSS (version now on GitHub). If it's publicly readable, then the HTML file is returned directly to the browser without a login for both pod/inbox and pod/inbox.html. If it's not publicly readable, then index.html doesn't get automatically rendered within solidos. The user has to actually navigate to the malicious html file within solidos. These are very subtle mitigations though, so I could imagine a regression occurring at some point.
  • In the case of the inbox, it's worth noting that it should only accept RDF, and that CSS already provides a (fairly recent) way of doing this as part of broader shape validation support: https://github.com/CommunitySolidServer/shape-validator-component/blob/main/documentation/constrained-containers.md#impact-of-constraining-a-container

@Otto-AA
Copy link
Author

Otto-AA commented Mar 18, 2023

Good point. I absolutely agree that untrusted agents should not be permitted to write HTML/executable code to a pod. This not something that solidos can solve alone, so an issue needs to be raised with NSS and CSS.

I partly agree. Either it should not be allowed, or afterwards sandboxed/sanitized/etc such that it does not execute.

However, my point is that it doesn't really matter if you're authenticated or not. If a user is authenticated and has access to /foo/bar I still don't want them to gain access over the whole pod. That's currently possible with NSS and the CSS mashlib recipe given this iframe issue for NSS and some other issues coming from not sandboxing for CSS. Also this works with very limited user interaction (clicking 1 link, given some other preconditions; for CSS I've made a private issue detailing it).

I could think of allowing the pod owner to create executable files. They probably don't gain anything from privilege escalation or other impacts. But authenticated agents with access to some resources should imo not be allowed to create executable files, at least not the way it is currently.

(And on a side note, I think that's not really related to this issue, so we should discuss somewhere else?)

@josephguillaume
Copy link
Contributor

on a side note, I think that's not really related to this issue, so we should discuss somewhere else

The relevance is that the ability of a user to access the parent window can possibly be useful, and so mitigations other than locking down an IFrame should be considered/HTML should not necessarily be locked down in every case. Iframe!= XSS, IFrame= risk of XSS.

In any case I don't have anything more to add at this stage.

@Otto-AA
Copy link
Author

Otto-AA commented Mar 19, 2023

Yes I think this feature case is clear. I also think that tackling this iframe issue is not the priority for now, it's rather one thing to consider in the "(how) should pods serve html files" discussion, for both the security and feature side. Until this discussion is resolved, imo it doesn't make much sense to decide what to do with this iframe issue. So for now I'd wait until pod implementors give their point of view

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

2 participants