openssh unix dev - Jan 2019 - Is sshd supposed to interpret "{a,b}" brace expansions?

Hi,

the proposed fix for CVE-2019-6111 [1] adds file name validation to scp
to prevent the server from sending files that the client actually did
not request. Now, a consequence of that patch is that commands which
contain server-side brace expansions such as

    $ scp remote:'/etc/{passwd,group}' .
    error: unexpected filename: passwd

no longer work. Shell globs such as [abc], ?, *, and combinations
thereof still work fine, but {a,b} does not.

Is that a shortcoming of the patch? Or is it intended behavior?

I looked through various man pages, but I could not find any definite
statement about whether server-side brace expansion are supposed to work
on or not. Could someone please enlighten me?

Best regards,
Peter


[1] https://sintonen.fi/advisories/scp-name-validator.patch

Hello,
from what I understand, the brace expansion is not expanded in the
remote scp nor sshd, but in the remote shell (the remote command is run
inside of bash -c "command"). The debug line looks like this:

  Executing: program /usr/bin/ssh host rhel7.virt, user (unspecified),
command scp -v -f /etc/{passwd,group}

But what is actually executed is

  bash -c "scp -v -f /etc/{passwd,group}"

expanding to in the remote shell (in the above example bash) to

  scp -v -f /etc/passwd /etc/group


Therefore for this patch to work the same way will need also the
GLOB_BRACE flag to the glob().

Regards,
Jakub


On Wed, 2019-01-30 at 12:34 +0100, Peter Simons wrote:
> Hi,
> 
> the proposed fix for CVE-2019-6111 [1] adds file name validation to
> scp
> to prevent the server from sending files that the client actually did
> not request. Now, a consequence of that patch is that commands which
> contain server-side brace expansions such as
> 
>     $ scp remote:'/etc/{passwd,group}' .
>     error: unexpected filename: passwd
> 
> no longer work. Shell globs such as [abc], ?, *, and combinations
> thereof still work fine, but {a,b} does not.
> 
> Is that a shortcoming of the patch? Or is it intended behavior?
> 
> I looked through various man pages, but I could not find any definite
> statement about whether server-side brace expansion are supposed to
> work
> on or not. Could someone please enlighten me?
> 
> Best regards,
> Peter
> 
> 
> [1] https://sintonen.fi/advisories/scp-name-validator.patch
> 
> _______________________________________________
> openssh-unix-dev mailing list
> openssh-unix-dev at mindrot.org
> https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
-- 
Jakub Jelen
Software Engineer
Security Technologies
Red Hat, Inc.

On Wed, 30 Jan 2019, Peter Simons wrote:
> Hi,
> 
> the proposed fix for CVE-2019-6111 [1] adds file name validation to scp
> to prevent the server from sending files that the client actually did
> not request.
That's _a_ proposed fix, but not the one we used.

Ours is:

https://anongit.mindrot.org/openssh.git/patch/?id=391ffc4b9
> Now, a consequence of that patch is that commands which
> contain server-side brace expansions such as
> 
>     $ scp remote:'/etc/{passwd,group}' .
>     error: unexpected filename: passwd
> 
> no longer work. Shell globs such as [abc], ?, *, and combinations
> thereof still work fine, but {a,b} does not.
> 
> Is that a shortcoming of the patch? Or is it intended behavior?
It's basically an inevitability that some patterns will fail. In the
general case, there's no way for the client to know what rules the
server will use to expand the filename that is passed. Throw in quoting
conventions and it's even more of a mess. For this reason, our patch
includes a flag (-T) to disable the client-side checks.

-d

Jakub Jelen writes:

 > from what I understand, the brace expansion is not expanded in the
 > remote scp nor sshd, but in the remote shell (the remote command is
 > run inside of bash -c "command").

yes, you are right of course. Thank you for pointing that out.


Damien Miller writes:

 >> the proposed fix for CVE-2019-6111 [1] adds file name validation to
 >> scp [...]
 >
 > That's _a_ proposed fix, but not the one we used.
 >
 > Ours is: https://anongit.mindrot.org/openssh.git/patch/?id=391ffc4b9

I see. Thank you very much for the pointer.

Best regards
Peter