Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
initial draft #2
initial draft #2
Changes from 2 commits
a2e02c2
e9d624d
210674f
872c19e
d7e6535
9636996
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How to reproduce this security issue:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From how I understand, you imply that the problem is already this step of giving Append/Write access.
If yes, I'd disagree with this. If you meant something else, can you clarify it?
Append/Write access seems like a simple requirement for an attacker, for various reasons.
Firstly, if I give someone access to append pictures to my /cats container, I would not expect this to be a security issue. In fact, if the attacker could gain full control over the pod just from Append access to a container, this would render access control rather useless. The Solid protocol / implementations should ensure that access control holds up, even with malicious agents.
Secondly, any CSS instance that is publicly open for registration would give an attacker the opportunity to host malicious html files on the same origin (solidweb.me, etc.). For ESS it's the same (at least for storage.inrupt.com). For NSS, other storages are only on the same site, not origin.
And thirdly, even if I only give agents access that I completely trust: They can also get hacked. If that happens, the impact of this should be as contained as possible. So we should only give them access to what they really need, and it should be hard for them to gain more access.
EDIT: in security terms, getting more access based from some initial access is called a "privilege escalation". Serving html/... files allows for privilege escalation if the server does not prevent it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please clarify what you mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@csarven, if I interpret your haiku correctly, you seem to suggest that Solid access control could be boiled down to a simple allow list of WebIDs, which would give every WebID listed full access to the storage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<script>alert()</script>
in the literal of a concrete RDF syntax falls within the same realm of security concerns.Actual security concern: Sanitise content before injecting into DOM or have a shape that validates the payload (e.g., to block or clear
src
,data
, inline scripts, and so on.) so that the consumer doesn't inadvertently run stuff.That's of course to prevent..
the agent that was somehow given append or write access permissions - read: trusted - to a storage who then turned out to be an "attacker" or unwillingly added malicious code won't cause chaos. /s
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could separate RDF application security issues from HTML/SVG browser related security issues.
I definitely agree, that malicious RDF can be an issue and we need to be careful when interacting with this data. I've also written an article about this last year (which in retrospectively still makes important points, but maybe it is from a hardcore security perspective): https://github.com/Otto-AA/solid-security-basics.
This also goes beyond html injection: For instance, Penny blindly trusted the container data when making a recursive deletion, such that you could have tricked it to delete any other storage as well, not only child resources. So also for business logic in applications, it is important not to blindly trust RDF data.
However, I think we could keep this security issue separate from serving html files. I would see sanitization of RDF as a task for the application that uses and renders it (currently for web apps mostly). They fetch it and, if they want to render it, they should sanitize it.
For html files we cannot rely on apps to do this sanitization. If the HTML file is served publicly, the browser will render it and execute its malicious content. A way we could do this "sanitization" in browsers would be to tell it not to render the html, with CSP headers etc. Alternatively, maybe the server could return a html-rendering app that manually fetches the html and renders a sanitized version. However, in both of these cases, it would be the servers responsibility.
I think it would make sense to separate it with this distinction: security issues where apps are responsible and security issues where servers are responsible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you like to add a recommendation on sanitizing any RDF Literal before using it in the DOM?
This could be a separate PR