-
Notifications
You must be signed in to change notification settings - Fork 19
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
Server prevents walks to ".." #62
Comments
I think you are correct, a 9P2000.L server should allow this behavior. I believe this is one of the custom gVisor modifications we made for security reasons -- we knew our client would never use ".." and we wanted to prevent users from walking outside of our root directory if they compromised the sentry. |
Thanks for the context. To add context on my end, I ended up not needing to use To work around this, the files in question were already storing data that made reconstructing their path easy. (Similar to how Plan 9's network |
Oh -- I think we can totally fix this. gVisor doesn't use this library -- I forked this out of gVisor so p9 would get better usability (use |
Awesome! |
intro(5) says
And walk(5) says
I don't see any mention of this in diod's protocol document, but I do see it used in their tests:
https://github.com/chaos/diod/blob/9da28f911978957dbec251c653200db7a4dcad6e/tests/user/testopenfid.c#L72
Which leads me to believe the same requirement for 9P, still holds true for 9P2000.L.
However, even if
File
implementations handle a dot-dot request inside theirWalk
method,Server
explicitly forbids the request from being issued to theFile
.p9/p9/handlers.go
Line 1165 in 49c780c
calls:
p9/p9/handlers.go
Line 1183 in 49c780c
calls:
p9/p9/handlers.go
Lines 1065 to 1070 in 49c780c
Bypassing the check for
..
seems to work as expected (tested againstcmd\p9ufs
withClient
; even trying to escape the root resolves to just.
ofp9ufs
's-root
argument).However, I'm not familiar enough with how the library tracks fid references, so I'm not sure if such a simple exception to the name rules somehow breaks reference handling in some way.
The logic around here is concerning
p9/p9/handlers.go
Line 1146 in 49c780c
since
newRef
should actually bewalkRef
's parent orwalkRef
itself ifwalkRef
is the root; but we addnewRef
as a child ofwalkRef
.doWalk
currently handles the case for clones (nil names), and for steps (some string name).But if the logic for stepping isn't also valid for backtracking (".." names), support for that may have to be added.
I'm not sure.
The text was updated successfully, but these errors were encountered: