Tab-completion problem through ssh when files start by dash (_remote_files:compadd:80: bad option: -@)

Tab-completion problem through ssh when files start by dash (_remote_files:compadd:80: bad option: -@)

[Antoine Amarilli]

[-- Attachment #1: Type: text/plain, Size: 1603 bytes --]

Hello everyone,

I would like to report what I think may be a bug in zsh. (I am not
subscribed directly, so please include me in any replies.) The bug
concerns tab-completion through ssh in directories containing a file
that starts with a dash.

The bug occurs when running zsh -f (using a fresh user on a Debian
testing amd64 machine, if that matters -- I call the machine "foo"). To
reproduce:

1. Enable autocomplete on machine foo by issuing:

  foo% autoload -Uz compinit
  foo% compinit

2. On a remote machine bar that can be reached by ssh with public-key
authentication from foo, in the home directory of user jdoe, create a
file whose name starts with a dash and occurs early in the alphabet, for
instance "-@":
  
  bar% cd ~jdoe
  bar% touch -- -@

3. Try to scp a file "blah" from foo to bar and use tab-completion
(indicated as <TAB>):

  foo% touch blah
  foo% scp blah jdoe@bar:<TAB>

When I do this, the result looks like this:

  foo% touch blah
  foo% scp blah jdoe@bar:<TAB>
  _remote_files:compadd:80: bad option: -@
  foo% scp test jdoe@bar:
  [ACTUAL CONTENTS OF ~jdoe ON bar]

So it looks to me like the internals of tab-completion are not properly
escaping the file names in this case, hence the warning. This is mostly
an annoyance, but maybe there could be some more problematic
implications (e.g., maybe a malicious jdoe on bar could create files
that would pass actual options to compadd and mess up more seriously
with the zsh session on foo).

I hope that this report is useful! :)

Best regards,

-- 
Antoine Amarilli


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Tab-completion problem through ssh when files start by dash (_remote_files:compadd:80: bad option: -@)

[Daniel Shahaf]

Antoine Amarilli wrote on Thu, Apr 13, 2017 at 19:47:17 +0200:
> So it looks to me like the internals of tab-completion are not properly
> escaping the file names in this case, hence the warning. This is mostly
> an annoyance, but maybe there could be some more problematic
> implications (e.g., maybe a malicious jdoe on bar could create files
> that would pass actual options to compadd and mess up more seriously
> with the zsh session on foo).

The «-R remote-func» option seems to be the most obvious method of
injection.  I'm not sure whether it requires a literal function name, or
whether an anonymous function would be accepted too.

> When I do this, the result looks like this:
> 
>   foo% touch blah
>   foo% scp blah jdoe@bar:<TAB>
>   _remote_files:compadd:80: bad option: -@
>   foo% scp test jdoe@bar:
>   [ACTUAL CONTENTS OF ~jdoe ON bar]

I think this fixes it?

diff --git a/Completion/Unix/Type/_remote_files b/Completion/Unix/Type/_remote_files
index 1e9fed1..a5fce9a 100644
--- a/Completion/Unix/Type/_remote_files
+++ b/Completion/Unix/Type/_remote_files
@@ -58,11 +58,13 @@ if zstyle -T ":completion:${curcontext}:files" remote-access; then
     else rempat="${(q)PREFIX%%[^./][^/]#}\*"
   fi
 
+  # remote filenames
   remfiles=(${(M)${(f)"$(_call_program files $cmd $cmd_args $host ls -d1FL -- "$rempat" 2>/dev/null)"}%%[^/]#(|/)})
 
   compset -P '*/'
   compset -S '/*' || (( ${args[(I)-/]} )) || suf='remote file'
 
+  # display strings for remote files and directories
   remdispf=(${remfiles:#*/})
   remdispd=(${(M)remfiles:#*/})
 
@@ -77,9 +79,9 @@ if zstyle -T ":completion:${curcontext}:files" remote-access; then
   while _tags; do
     while _next_label files expl ${suf:-remote directory}; do
       [[ -n $suf ]] &&
-          compadd "$args[@]" "$expl[@]" -d remdispf ${(q)remdispf%[*=|]} && ret=0
+          compadd "$args[@]" "$expl[@]" -d remdispf -- ${(q)remdispf%[*=|]} && ret=0
       compadd ${suf:+-S/} -r "/ \t\n\-" "$args[@]" "$expl[@]" -d remdispd \
-	${(q)remdispd%/} && ret=0
+	-- ${(q)remdispd%/} && ret=0
     done
     (( ret )) || return 0
   done

Thanks for the report!

Daniel

Re: Tab-completion problem through ssh when files start by dash (_remote_files:compadd:80: bad option: -@)

[Bart Schaefer]

On Apr 15,  1:19am, Daniel Shahaf wrote:
}
} The "-R remote-func" option seems to be the most obvious method of
} injection.  I'm not sure whether it requires a literal function name, or
} whether an anonymous function would be accepted too.

It's remove-func, not remote-func ... and it does have to be a literal
function name, it is not "eval"'d.

Re: Tab-completion problem through ssh when files start by dash (_remote_files:compadd:80: bad option: -@)

[Antoine Amarilli]

[-- Attachment #1: Type: text/plain, Size: 1468 bytes --]

Hi Daniel,

Thanks for your reply.

On Sat, Apr 15, 2017 at 01:19:02AM +0000, Daniel Shahaf wrote:
> Antoine Amarilli wrote on Thu, Apr 13, 2017 at 19:47:17 +0200:
> > So it looks to me like the internals of tab-completion are not properly
> > escaping the file names in this case, hence the warning. This is mostly
> > an annoyance, but maybe there could be some more problematic
> > implications (e.g., maybe a malicious jdoe on bar could create files
> > that would pass actual options to compadd and mess up more seriously
> > with the zsh session on foo).
> 
> The «-R remote-func» option seems to be the most obvious method of
> injection.  I'm not sure whether it requires a literal function name, or
> whether an anonymous function would be accepted too.

I played a bit with it but wasn't able to get it to execute. That said,
I'm not at all familiar with the semantics of compadd, so someone more
familiar who can reproduce the problem may be able to achieve
something...

> I think this fixes it?
> 
> diff --git a/Completion/Unix/Type/_remote_files b/Completion/Unix/Type/_remote_files
> index 1e9fed1..a5fce9a 100644

I patched my copy of
/usr/share/zsh/functions/Completion/Unix/_remote_files following this
diff, and indeed this silences the warning and tab-completion seems to
work. Thanks!

I guess it would be good to commit this fix in the codebase then?

Thanks a lot again!

Best,

-- 
Antoine Amarilli


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Tab-completion problem through ssh when files start by dash (_remote_files:compadd:80: bad option: -@)

[Daniel Shahaf]

Antoine Amarilli wrote on Sun, Apr 16, 2017 at 21:08:31 +0200:
> I guess it would be good to commit this fix in the codebase then?

Yeah, I'll commit it in a couple of days.

Cheers,

Daniel