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@

diff --git a/usr.bin/ssh/scp.c b/usr.bin/ssh/scp.c
index 582e232..c7194c2 100644 (file)
--- a/usr.bin/ssh/scp.c
+++ b/usr.bin/ssh/scp.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: scp.c,v 1.248 2022/05/13 06:31:50 djm Exp $ */
+/* $OpenBSD: scp.c,v 1.249 2022/10/24 21:51:55 djm Exp $ */
 /*
  * scp - secure remote copy.  This is basically patched BSD rcp which
  * uses ssh to do the data transfer (instead of using rcmd).
@@ -1474,7 +1474,8 @@ sink_sftp(int argc, char *dst, const char *src, struct sftp_conn *conn)
        }
 
        debug3_f("copying remote %s to local %s", abs_src, dst);
-       if ((r = remote_glob(conn, abs_src, GLOB_MARK, NULL, &g)) != 0) {
+       if ((r = remote_glob(conn, abs_src, GLOB_NOCHECK|GLOB_MARK,
+           NULL, &g)) != 0) {
                if (r == GLOB_NOSPACE)
                        error("%s: too many glob matches", src);
                else
@@ -1483,6 +1484,20 @@ sink_sftp(int argc, char *dst, const char *src, struct sftp_conn *conn)
                goto out;
        }
 
+       /* Did we actually get any matches back from the glob? */
+       if (g.gl_matchc == 0 && g.gl_pathc == 1 && g.gl_pathv[0] != 0) {
+               /*
+                * If nothing matched but a path returned, then it's probably
+                * a GLOB_NOCHECK result. Check whether the unglobbed path
+                * exists so we can give a nice error message early.
+                */
+               if (do_stat(conn, g.gl_pathv[0], 1) == NULL) {
+                       error("%s: %s", src, strerror(ENOENT));
+                       err = -1;
+                       goto out;
+               }
+       }
+
        if ((r = stat(dst, &st)) != 0)
                debug2_f("stat local \"%s\": %s", dst, strerror(errno));
        dst_is_dir = r == 0 && S_ISDIR(st.st_mode);
@@ -1700,7 +1715,8 @@ sink(int argc, char **argv, const char *src)
                }
                if (npatterns > 0) {
                        for (n = 0; n < npatterns; n++) {
-                               if (fnmatch(patterns[n], cp, 0) == 0)
+                               if (strcmp(patterns[n], cp) == 0 ||
+                                   fnmatch(patterns[n], cp, 0) == 0)
                                        break;
                        }
                        if (n >= npatterns)
@@ -1883,7 +1899,8 @@ throughlocal_sftp(struct sftp_conn *from, struct sftp_conn *to,
        }
 
        debug3_f("copying remote %s to remote %s", abs_src, target);
-       if ((r = remote_glob(from, abs_src, GLOB_MARK, NULL, &g)) != 0) {
+       if ((r = remote_glob(from, abs_src, GLOB_NOCHECK|GLOB_MARK,
+           NULL, &g)) != 0) {
                if (r == GLOB_NOSPACE)
                        error("%s: too many glob matches", src);
                else
@@ -1892,6 +1909,20 @@ throughlocal_sftp(struct sftp_conn *from, struct sftp_conn *to,
                goto out;
        }
 
+       /* Did we actually get any matches back from the glob? */
+       if (g.gl_matchc == 0 && g.gl_pathc == 1 && g.gl_pathv[0] != 0) {
+               /*
+                * If nothing matched but a path returned, then it's probably
+                * a GLOB_NOCHECK result. Check whether the unglobbed path
+                * exists so we can give a nice error message early.
+                */
+               if (do_stat(from, g.gl_pathv[0], 1) == NULL) {
+                       error("%s: %s", src, strerror(ENOENT));
+                       err = -1;
+                       goto out;
+               }
+       }
+
        for (i = 0; g.gl_pathv[i] && !interrupted; i++) {
                tmp = xstrdup(g.gl_pathv[i]);
                if ((filename = basename(tmp)) == NULL) {

diff --git a/usr.bin/ssh/sftp-glob.c b/usr.bin/ssh/sftp-glob.c
index ee1d123..3273091 100644 (file)
--- a/usr.bin/ssh/sftp-glob.c
+++ b/usr.bin/ssh/sftp-glob.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sftp-glob.c,v 1.30 2022/02/25 09:46:24 dtucker Exp $ */
+/* $OpenBSD: sftp-glob.c,v 1.31 2022/10/24 21:51:55 djm Exp $ */
 /*
  * Copyright (c) 2001-2004 Damien Miller <djm@openbsd.org>
  *
@@ -110,6 +110,11 @@ int
 remote_glob(struct sftp_conn *conn, const char *pattern, int flags,
     int (*errfunc)(const char *, int), glob_t *pglob)
 {
+       int r;
+       size_t l;
+       char *s;
+       struct stat sb;
+
        pglob->gl_opendir = fudge_opendir;
        pglob->gl_readdir = (struct dirent *(*)(void *))fudge_readdir;
        pglob->gl_closedir = (void (*)(void *))fudge_closedir;
@@ -119,5 +124,30 @@ remote_glob(struct sftp_conn *conn, const char *pattern, int flags,
        memset(&cur, 0, sizeof(cur));
        cur.conn = conn;
 
-       return(glob(pattern, flags | GLOB_ALTDIRFUNC, errfunc, pglob));
+       if ((r = glob(pattern, flags | GLOB_ALTDIRFUNC, errfunc, pglob)) != 0)
+               return r;
+       /*
+        * When both GLOB_NOCHECK and GLOB_MARK are active, a single gl_pathv
+        * entry has been returned and that entry has not already been marked,
+        * then check whether it needs a '/' appended as a directory mark.
+        *
+        * This ensures that a NOCHECK result is annotated as a directory.
+        * The glob(3) spec doesn't promise to mark NOCHECK entries, but doing
+        * it simplifies our callers (sftp/scp) considerably.
+        *
+        * XXX doesn't try to handle gl_offs.
+        */
+       if ((flags & (GLOB_NOCHECK|GLOB_MARK)) == (GLOB_NOCHECK|GLOB_MARK) &&
+           pglob->gl_matchc == 0 && pglob->gl_offs == 0 &&
+           pglob->gl_pathc == 1 && (s = pglob->gl_pathv[0]) != NULL &&
+           (l = strlen(s)) > 0 && s[l-1] != '/') {
+               if (fudge_stat(s, &sb) == 0 && S_ISDIR(sb.st_mode)) {
+                       /* NOCHECK on a directory; annotate */
+                       if ((s = realloc(s, l + 2)) != NULL) {
+                               memcpy(s + l, "/", 2);
+                               pglob->gl_pathv[0] = s;
+                       }
+               }
+       }
+       return 0;
 }