Open dundalek opened 6 years ago
A response from fish-shell gitter:
It's a consequence of how completions work.If you type that command line at a prompt and press
the same thing will happen. I'm curious about your example and why you're trying to complete that string. The $() construct isn't currently valid in fish. That's POSIX shell syntax. Furthermore, it's not a valid command line so there won't be any matching completions. So your example is nonsensical unless you're deliberately testing what happens when the completion logic is handed an invalid command line.
Can it be abused for some attacks?
Only if you can get the user to install and run your script. And if you can get them to do that there are easier ways of causing mischief.
So I am wondering whether this even needs fixing and have currently no idea what the fix would be.
I wouldn't say this can be used for attacks, and it doesn't seem like that big of a problem. The only way this might be harmful is that it is confusing and mostly unexpected.
Some tests I did in closh/bash/fish (not zsh):
echo $(touch /tmp/test1)
-- closh through bash completionecho `touch /tmp/test2`
-- closh through bash completionecho (touch /tmp/test3)
-- none (this is command substitution syntax for fish)$(touch /tmp/test4)
-- fish, closh through fish and bash completion`touch /tmp/test5`
-- closh through bash completion(touch /tmp/test3)
-- fish, closh through fish and bash completion (sub-shell syntax)When completing in bash, not a single one of these examples resulted in execution, at least for my configuration.
For fish with default configuration, it's only with 4 and 6.
Note that n6 (command)
seems to be valid in bash through sub-shells, hinting that {command}
for code blocks might also be problematic (I haven't tested.)
As said, you'ld have to deliberately come up with something that is invalid syntax. In the case of closh that is the form where you try to use command substitution ($(command)
or `command`
), so that it can be interpreted by the completion loader. But note nr 6: the (function)
is most definitely valid closh syntax, but sadly the completions also execute them as (command)
, thus (cat)<TAB>
hangs. For me the only real issue here seems to be the implementation of the completion scripts. From my limited experience I know it's very easy to get these things wrong. (It really surprises me the fish gitter seem to be okay with this, though.)
I really don't know enough about shell design, completion, etc. to be able to say if it's feasible or not, but it seems to me that completers should only ever consider raw text (and possibly variables), but never the results of commands substitutions. This is something the bash (zsh?) completion script most definitely doesn't adhere to while the bash shell does seem to. If it is at all possible to fix that, it would already solve 4/6 of above simple test cases. As for the fish completion, fixing is definitely a job for upstream (if not intended behavior).
Typing something in closh like
$(touch /tmp/test)
and pressing tab for completion will result in that code being executed (!).It seems to affect completions for all shells: bash, zsh and even fish.
Kudos to @TGThorax