openssh bugs - Oct 2022 - [Bug 3488] New: scp fails to locate remote paths containing a non-empty square bracket pair

https://bugzilla.mindrot.org/show_bug.cgi?id=3488

            Bug ID: 3488
           Summary: scp fails to locate remote paths containing a
                    non-empty square bracket pair
           Product: Portable OpenSSH
           Version: 9.1p1
          Hardware: amd64
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P5
         Component: scp
          Assignee: unassigned-bugs at mindrot.org
          Reporter: burnsmellfactory at gmail.com

I'm noticing this issue in OpenSSH 9.0p1 and 9.1p1.

If the remote path contains a matched square bracket pair (i.e. '['
followed sometime later by ']'), and there is any text between the
opening and closing square brackets, scp fails to locate that path.

If I have the following files on a remote server:
A) foo[]
B) foo]0[
C) foo[0
D) foo0]
E) foo[0]
F) foo[x]
G) bar[/foo]
H) bar[/]foo

And on a client I try to scp them like so:
A) scp -pr "user at host:/path/foo[]" .
B) scp -pr "user at host:/path/foo]0[" .
C) scp -pr "user at host:/path/foo[0" .
D) scp -pr "user at host:/path/foo0]" .
E) scp -pr "user at host:/path/foo[0]" .
F) scp -pr "user at host:/path/foo[x]" .
G) scp -pr "user at host:/path/bar[/foo]" .
H) scp -pr "user at host:/path/bar[/]foo" .

A, B, C, and D will succeed.  E, F, G, and H will all fail with the
error:
scp: /path/...: No such file or directory
And the path in the error message will match exactly the path that does
in fact exist.

As you can see with case H, the pair doesn't have to occur within a
single path component, it can occur anywhere across the entire path.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3488

burnsmellfactory at gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |burnsmellfactory at gmail.com

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3488

--- Comment #1 from burnsmellfactory at gmail.com ---
FYI the -pr options are just my habit when I run scp, it makes no
difference to the outcome of the test.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3488

--- Comment #2 from Michael Yagliyan <burnsmellfactory at gmail.com> ---
The only bug report I could find having to do with brackets was
https://bugzilla.mindrot.org/show_bug.cgi?id=1602.  I don't know enough
about scp and OpenSSH to know if it's actually related, just posting
here in case it is helpful to someone more knowledgeable.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3488

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dtucker at dtucker.net

--- Comment #3 from Darren Tucker <dtucker at dtucker.net> ---
What's happening is that the bracket pairs denote glob(3) patterns, and
examples E through H are patterns that do not match anything.  If you
add a file that does match the pattern you can see this:

$ touch path/foo0
$ scp -v 'localhost:path/foo[0]' /tmp
[...]
$ scp: debug1: Fetching path/foo0 to /tmp/foo0

and you can prefix the brackets with a backslash to prevent this:

$ scp -v 'localhost:path/foo\[0\]' /tmp
[...]
scp: debug1: Fetching path/foo[0] to /tmp/foo[0]

I suspect this might have worked with older scp versions because the
path would get passed to the shell, but since the glob didn't match
anything it would not be expanded and used literally.  It doesn't work
with scp's current fallback mode because the sent file ends up not
matching what the local scp thinks it requested and the defenses
against remote scp bogus path attacks block it:

$ scp -O 'localhost:path/foo[0]' /tmp
protocol error: filename does not match request

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3488

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned-bugs at mindrot.org |djm at mindrot.org
                 CC|                            |djm at mindrot.org
   Attachment #3619|                            |ok?(dtucker at dtucker.net)
              Flags|                            |

--- Comment #4 from Damien Miller <djm at mindrot.org> ---
Created attachment 3619
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3619&action=edit
handle glob no-match conditions like olde scp

Original rcp-protocol scp would use the shell for remote glob(3)
expansion, and shells usually fall back in the case where the glob
expansion fails to considering the original path literally.

We can do this in scp in SFTP protocol mode using remote_glob()'s
GLOB_NOCHECK flag, which instructs it to return the original path if
expansion fails.

However, we also depend on GLOB_MARK to annotate directory paths with a
'/' suffix, and that doesn't happen to work for the GLOB_NOCHECK
case.

So this changes three things:

1. Turns on GLOB_NOCHECK for the scp remote_glob() calls

2. Adds support to sftp-glob.c:remote_glob() to properly annotate
returned GLOB_NOCHECK paths when GLOB_MARK is enabled

3. Adds existence checks after remote_glob() when a GLOB_NOCHECK result
is returned so it can show a consistent error message for actually
missing files (rather than letting it fall through to the do_download()
call which doesn't do nice error messages).

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3488

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3619|ok?(dtucker at dtucker.net)    |ok+
              Flags|                            |

--- Comment #5 from Darren Tucker <dtucker at dtucker.net> ---
Comment on attachment 3619
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3619
handle glob no-match conditions like olde scp

should this have a regress test?

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3488

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Blocks|                            |3480
             Status|NEW                         |RESOLVED

--- Comment #6 from Damien Miller <djm at mindrot.org> ---
These are fixed in OpenSSH HEAD and will be in openssh-9.2.

commit 7190154de2c9fe135f0cc1ad349cb2fa45152b89 (HEAD -> master)
Author: djm at openbsd.org <djm at openbsd.org>
Date:   Mon Oct 24 21:52:50 2022 +0000

    upstream: regress test for unmatched glob characters; fails before

    previous commit but passes now. bz3488; prodded by dtucker@

    OpenBSD-Regress-ID: 0cc5cc9ea4a6fd170dc61b9212f15badaafb3bbd

commit a4821a592456c3add3cd325db433110cdaaa3e5c
Author: djm at openbsd.org <djm at openbsd.org>
Date:   Mon Oct 24 21:51:55 2022 +0000

    upstream: when scp(1) is using the SFTP protocol for transport (the

    default), better match scp/rcp's handling of globs that don't match
the
    globbed characters but do match literally (e.g. trying to transfer
    "foo.[1]").

    Previously scp(1) in SFTP mode would not match these pathnames but
    legacy scp/rcp mode would.

    Reported by Michael Yagliyan in bz3488; ok dtucker@

    OpenBSD-Commit-ID: d8a3773f53015ba811fddba7473769a2fd343e11


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=3480
[Bug 3480] tracking bug for openssh-9.2
-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3488

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #7 from Damien Miller <djm at mindrot.org> ---
OpenSSH 9.3 has been released. Close resolved bugs

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.