-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
WebUI should (somehow) indicate a problematic directory entry #4292
Comments
We should definitely be removing the duplicate entries. As for the security implications, this definitely isn't a good bug to have but would probably be hard to exploit. The (really) worrying part is that you didn't get a timely response; I've opened up an internal discussion to figure out what happened there. Note: That part of the IPLD spec is only tangentially related. IPFS directories have a very different structure. |
Only if there are guarantees within all readers ( http, cat, fuse ) that the first encountered "named link" is the one that wins. Otherwise the problem will only get worse. Stepping back a bit - I was really hoping for actual marking of all duplicates or semi-duplicates as "Hey! Something is fishy". The thing I constructed is a simple bit-for-bit duplication. However, when you consider all the nastiness within #1710, just hiding an offender is likely insufficient, as it will miss:
It's growing pains within the team, it is understandable. If anything - this is a relatively low profile issue, so having the report-loop tested with that is way better than some sort of RCE ;) |
This sounds like terminal control characters might go through as well. Maybe worth a try. A program that prints non-binary to stdout should never print special control characters, partly because of this, but also because it's sort-of an injection problem if stdout is piped to stdin of another program that expects a line separated "protocol". I usually never print user input without Regarding duplicate entries: I think the |
@kpcyrd I didn't even think to test this, but yes ( example outputs color codes only, is safe to
|
@Stebalien All the problems with non-printables aside - there is also a length-based DoS: http://ipfs.io/ipfs/QmVtFwE9CWfBk7WC2pCgPtMQsZ7zF6tgjbL5Vcpte7pLcy Trying to follow the longer-named links results in either a 414 ( decent ) or 502 ( ugh ) on the gateway. On the CLI things are no better:
|
We'd do this at the lowest level so yes, it would cover all cases. I'd like to just reject such directories outright but directory sharding makes that impossible.
I'd like to handle this stuff at a layer below the UI. Users should (almost) never be asked to deal with security related things because they'll generally just be confused.
So, we initially wanted Unix compatibility but, as you can see in that thread, sentiments have changed. Limiting unixfs paths to printable, UTF-8 byte sequences is reasonable.
For long paths, we've considered limiting the length of individual components in a path (not entire paths, just components). However, we haven't picked a good limit. Currently, there's a hard limit of slightly less than 1MiB (due to other constraints) but we almost definitely want something shorter (4096?). Gateways can choose to reject any paths they want and we should probably set a "paths longer than X may not work, avoid them" limit. As for large directories, that's something we can't fix. We want to, e.g., support dumping Wikipedia into IPFS (and every article on Wikipedia lives in If you're feeling adventurous, want to read through #1710 and take a stab at a reasonable path restriction proposal? |
I have read it, multiple times: I was the one who linked it into the thread ;)
I currently can't commit to that, due to severe time shortage ( sorry :/ ). I will only make a rather off-the-cuff remark about something I think has not been explicitly explored: It seems that the question of "what fundamentally is the UnixFS portion of IPFS, and what naming rules apply" remains unresolved ( that's really a question for @jbenet and friends )
A different way of asking the above question is:
An explicit answer to this will inform where the rest of this ticket goes. Furthermore this answer must be provided and codified sooner rather than later, due to the accelerating growth of 3rd party implementations |
@mib-kd743naq my question is: it it bad that this can happen? and how does for example |
@Kubuxu the problem is that I do not know :/ Intuitively it is a rather nasty situation when "input arriving from the internet" is able to manipulate your cursor position and essentially redraw the entirety of the terminal. There is something about IPFS being directly connected to the net that makes the scenario less palatable than say a Not having a clear and well thought through answer to "is this a problem", is why I sidestepped this question entirely, and instead put together the "which one of the two" writeup above. It seems clearer from that standpoint that IPFS can't be both "fully POSIX compliant" and "an upgraded protocol with less cruft". For me personally - "Fully POSIX compliant" is the answer, with the ( significant ) work needed to make the tooling behave better. But I really want to defer this to @jbenet in part because of his proven visionary track record, and in part because he has extensively thought through this already ( or so I hope ).
Way better than I expected:
|
As we are working on ipld/legacy-unixfs-v2#1, if the decision is to allow all link names then best part forward (I think) would be to also to spec out way of escaping them for the user.
I disagree with this statement, what is stored (and can be accessed through the API) versus what is presented to the user are two things that can be different. As most of shells are not about to handle binary strings (as variables) it makes sense to have encoding in presenting them. |
@Kubuxu in light of
does my statement about "IPFS/IPLD can't be both at the same time" make more sense? |
Version information:
Any
Type:
Enhancement
( arguablyBug
)Severity:
Medium
Description:
When faced with a duplicate name in a directory, the UI does not signal in any way that something is amiss. There should be some sort of indication ( a red exclamation mark or something ) to visually alert the user "hey, watch out". I am not able to build FUSE locally, so I do not know how things behave there: likely needs to be investigated as well.
Here is an example DAG: https://ipfs.io/ipfs/zdjA8bKXBUttGJ7aXtG3DeuHYtAVXNj1jA5sKuscTJ1nk1xL9
This issue is tangentially related to #1710. There is also a somewhat explicit policy in the IPLD spec, but if the UI doesn't show anything about it - this is still a problem.
P.S. In case this is deemed a security issue that should not be public - feel free to delete this bugreport. I tried to use the
security@
contact two times, but never received a response, so decided to file here before I move on.The text was updated successfully, but these errors were encountered: