From 512d1b9c093dec2e517136480139bcf89931ab3a Mon Sep 17 00:00:00 2001 From: djm Date: Mon, 24 Oct 2022 21:51:55 +0000 Subject: [PATCH] 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@ --- usr.bin/ssh/scp.c | 39 +++++++++++++++++++++++++++++++++++---- usr.bin/ssh/sftp-glob.c | 34 ++++++++++++++++++++++++++++++++-- 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/usr.bin/ssh/scp.c b/usr.bin/ssh/scp.c index 582e2327ba0..c7194c247df 100644 --- 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 ee1d123d5c7..32730913bcf 100644 --- 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 * @@ -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; } -- 2.20.1