Mis handling of percent encoded paths

[damooo]

Currently

1. NSS raises internal server error, if slug of a non-container resource ends with "%2F", which is also pct-encoded alias of "/". Thus we cannot do PUT res with uri like http://example.org/pod1/c1/c2/t1%2f with any content-type other than txt/html. NSS will raise internal server error. Even with txt/html, it creates container t1/ instead of non-container t1%2F resource
2. When slug contains %2F in middle, like t1%2Faa.txt, server sends success status on put request, and correctly returns location as t1%2Faa.txt under it's parent container, and also sends correct acl location. But in actual it instead creates t1/aa.txt resource, along with t1/ container.

The problem seems, when translating url-path to fs-path, it pct-decodes entire path instead of splitting path segments, and decoding them safely and combining them back. This violates uri standard too.

ESS sends 403 instead of 500, for all above cases, even though there is no spec violation.

CSS seems also has issue

[jeff-zucker]

The slug is only a suggestion to the server of the name of a resource. What link header are you sending? If you are sending Container than the trailing slash is not needed and if you are sending Resource the trailing slash is invalid.

[damooo]

I should use word segment may be. not slug. If last path segment of resource uri contains %2F, then put on it behaves as in above cases. %2F in segment is not same as having / in path.

[damooo]

Hello @jeff-zucker , Bug is serious enough that, one can bypass all acls to read/write a resource, if that person has access for that operation at any of parent containers. i.e. Even if custom acls are specified for a descendent, one can bypass them.

For e.g. one can do the following.

1. give acl:read by _ acl:agentClass foaf:Agent on resource /home/, and also set the acl as default for it's descendents by acl:default. Thus container /home/ and it's descendents are public readable.
2. create /home/private/, and create custom acls for the container. set custom acls such that this container and it's descendents can't be read by public. Thus container /home/private/ is not public readable
3. now create a resource home/private/password.txt. It is rightly not public readable if we access it by url https:://pod.solidcommunity.net/home/private/password.txt, assuminig https:://pod.solidcommunity.net/ as storage root.
4. But one can easily access it by going to url https:://pod.solidcommunity.net/home/private%2fpassword.txt, effectively bypassing acls.

This case is not limited to read operation, or public agent, but any of them.

The issue is much general, that cause many many other issues like previous comment.

[jeff-zucker]

When you set the permissions on a container, you are only setting the permissions on the container listing, not on the contents of the container. If you want the contents of the container to follow the same permissions as the container listing, you need to put acl:default <./> in the container's .acl. This means that the permissions on the container are now the default for the contents of the container as well.

[damooo]

@jeff-zucker , i edited steps to explicitly mention that part. Isuue is still the same

[bourgeoa]

@jeff-zucker the issue seems that running up the tree for ACL from https://pod.solidcommunity.net/home/private%2fpassword.txt check for https://pod.solidcommunity.net/home/.acl which is correct but https://pod.solidcommunity.net/home/private%2fpassword.txt should fail with 404.

I'm wondering if the issue is in SolidOS because I have the same problem with CSS

[bourgeoa]

@damooo can you confirm you are doing your tests with the SolidOS web app ?

[jeff-zucker]

Ah, I see now. @damooo - yes pretty serious, thanks for reporting.

I note that going to https://pod/public%2f in the browser, redirects me to https://pod/public%2F/ - note it both capitalizes the F and it adds the slash. Huh?

[jeff-zucker]

^^And SolidOS shows me the https//pod/public/ container even though the URL bar now shows https://pod/public%2F/.

[damooo]

@bourgeoa , issue happens with css too, as it to has same bug as mentioned in initial comment.

One can reliably reproduce it with curl. Issue is with uri-normalization and uri-equivalance. There are pretty serious things we can do. currently we can create/read/delete resources which are otherwise non-accessible. And we can cause many internal-server-errors. If it is mixed with other uri-normalization patterns, we can even write/(atleast corrupt) to some one else's pod easily, if only we have .permissions for our own pod in shared server.

There seems also possible, we can exploit uri-equivalance issues. by just pct aliasing un-reserved-chars/gen-delims(even non-slash)/sub-delims, we can invalidate an acl easily.

[bourgeoa]

@timbl @RubenVerborgh @joachimvh I can confirm that the problem exist in NSS:latest and CSSv3.0.0 using CURL

# curl "https://solidweb.me/bourgeoa/testfolder/home%2Ftest.txt"
testtesttest

# curl "https://solidweb.me/bourgeoa/testfolder/home/test.txt"
{"name":"UnauthorizedHttpError","message":"","statusCode":401,"errorCode":"H401"

I NSS location of the convertions are in https://github.com/solid/node-solid-server/blob/2f679c0feea407c5ef6352481ea0e919d465821d/lib/resource-mapper.js#L86 And https://github.com/solid/node-solid-server/blob/2f679c0feea407c5ef6352481ea0e919d465821d/lib/resource-mapper.js#L98

The last one is the problem. Should %2F be forbidden in solid URL (per solid specification) due to the special nature of '/' or make a special function to replace decodeURIComponent with no decode on %2F

[damooo]

@bourgeoa

From rfc3986

URIs that differ in the replacement of a reserved character with its
   corresponding percent-encoded octet are not equivalent.  Percent-
   encoding a reserved character, or decoding a percent-encoded octet
   that corresponds to a reserved character, will change how the URI is
   interpreted by most applications.  Thus, characters in the reserved
   set are protected from normalization

/ is a gen-delim, and hence reserved. It is invalid to use decodeURIComponent on whole path.

Also from spec:

When a URI is dereferenced, the components and subcomponents
   significant to the scheme-specific dereferencing process (if any)
   must be parsed and separated before the percent-encoded octets within
   those components can be safely decoded, as otherwise the data may be
   mistaken for component delimiters

Thus, proper way would be, first seperate segments by /, and do any translation on path-segments, and then concat.

[joachimvh]

I did some tests on CSS 3.0.

Everything seems to work correctly on the memory backend, but there are issues with the file backend. I will open an issue in the CSS repo.

• PUT http://localhost:3000/pod1/c1/c2/t1%2f -> Throws an error.
• POST http://localhost:3000/ with slug t1%2Faa -> Throws an error.
• PUT http://localhost:3000/foo/bar -> Accessible through http://localhost:3000/foo%2fbar

ACL paths when accessing http://localhost:3000/foo/bar

http://localhost:3000/foo/bar
http://localhost:3000/foo/
http://localhost:3000/

ACL paths when accessing http://localhost:3000/foo%2fbar

http://localhost:3000/foo%2Fbar
http://localhost:3000/

So making sure the file backend correctly interprets http://localhost:3000/foo/bar and http://localhost:3000/foo%2fbar as separate resources should fix the issue there.

[RubenVerborgh]

I think we want to escalate this to spec level; tagging @csarven. We might want to be cautious and disallow%2f—if not, we should be very clear on how to handle it, given the special semantics of slashes.

[jeff-zucker]

@bourgeoa - I am concerned about this commit. For erxample this is what the webapp does :

https://solid.github.io/mashlib/dist/browse.html?uri=https%3A%2F%2Fjeff-zucker.solidcommunity.net%2F

What will happen to the %2F in the query string?

[RubenVerborgh]

Rejecting might not be the right option, we might want to treat them just like normal slashes: https://github.com/solid/community-server/issues/1184#issuecomment-1059840423 Also because some proxies might decode.

[RubenVerborgh]

BTW We should be careful with interpreting RFCs in the right context.

The URI spec RFC3986 indeed says

URIs that differ in the replacement of a reserved character with its corresponding percent-encoded octet are not equivalent.

and we know that http://example.org/foo/bar and http://example.org/foo%2Fbar and http://example.org/foo%2fbar are considered different identifiers within RDF.

However, that is purely from a URI perspective; any server implementation is allowed to make further assumptions.

[bourgeoa]

@jeff-zucker It is rejected at the mapping between URL and file. There is no query string at this level. Tested locally. For NSS this is a draft PR for discussion and tests.

[bourgeoa]

solved in https://github.com/nodeSolidServer/node-solid-server/pull/1695