Date: Tue, 14 May 2019 22:54:17 +0000 (UTC) From: Gordon Tetlow <gordon@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-releng@freebsd.org Subject: svn commit: r347586 - releng/12.0/crypto/openssh Message-ID: <201905142254.x4EMsH6O047905@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: gordon Date: Tue May 14 22:54:17 2019 New Revision: 347586 URL: https://svnweb.freebsd.org/changeset/base/347586 Log: Fix insufficient filename validation in scp client Approved by: so Security: FreeBSD-EN-19:10.scp Modified: releng/12.0/crypto/openssh/scp.1 releng/12.0/crypto/openssh/scp.c Modified: releng/12.0/crypto/openssh/scp.1 ============================================================================== --- releng/12.0/crypto/openssh/scp.1 Tue May 14 22:51:49 2019 (r347585) +++ releng/12.0/crypto/openssh/scp.1 Tue May 14 22:54:17 2019 (r347586) @@ -18,7 +18,7 @@ .Nd secure copy (remote file copy program) .Sh SYNOPSIS .Nm scp -.Op Fl 346BCpqrv +.Op Fl 346BCpqrTv .Op Fl c Ar cipher .Op Fl F Ar ssh_config .Op Fl i Ar identity_file @@ -207,6 +207,16 @@ to use for the encrypted connection. The program must understand .Xr ssh 1 options. +.It Fl T +Disable strict filename checking. +By default when copying files from a remote host to a local directory +.Nm +checks that the received filenames match those requested on the command-line +to prevent the remote end from sending unexpected or unwanted files. +Because of differences in how various operating systems and shells interpret +filename wildcards, these checks may cause wanted files to be rejected. +This option disables these checks at the expense of fully trusting that +the server will not send unexpected filenames. .It Fl v Verbose mode. Causes Modified: releng/12.0/crypto/openssh/scp.c ============================================================================== --- releng/12.0/crypto/openssh/scp.c Tue May 14 22:51:49 2019 (r347585) +++ releng/12.0/crypto/openssh/scp.c Tue May 14 22:54:17 2019 (r347586) @@ -1,4 +1,4 @@ -/* $OpenBSD: scp.c,v 1.197 2018/06/01 04:31:48 dtucker Exp $ */ +/* $OpenBSD: scp.c,v 1.204 2019/02/10 11:15:52 djm Exp $ */ /* * scp - secure remote copy. This is basically patched BSD rcp which * uses ssh to do the data transfer (instead of using rcmd). @@ -94,6 +94,7 @@ #include <dirent.h> #include <errno.h> #include <fcntl.h> +#include <fnmatch.h> #include <limits.h> #include <locale.h> #include <pwd.h> @@ -375,14 +376,14 @@ void verifydir(char *); struct passwd *pwd; uid_t userid; int errs, remin, remout; -int pflag, iamremote, iamrecursive, targetshouldbedirectory; +int Tflag, pflag, iamremote, iamrecursive, targetshouldbedirectory; #define CMDNEEDS 64 char cmd[CMDNEEDS]; /* must hold "rcp -r -p -d\0" */ int response(void); void rsource(char *, struct stat *); -void sink(int, char *[]); +void sink(int, char *[], const char *); void source(int, char *[]); void tolocal(int, char *[]); void toremote(int, char *[]); @@ -421,8 +422,9 @@ main(int argc, char **argv) addargs(&args, "-oRemoteCommand=none"); addargs(&args, "-oRequestTTY=no"); - fflag = tflag = 0; - while ((ch = getopt(argc, argv, "dfl:prtvBCc:i:P:q12346S:o:F:")) != -1) + fflag = Tflag = tflag = 0; + while ((ch = getopt(argc, argv, + "dfl:prtTvBCc:i:P:q12346S:o:F:")) != -1) { switch (ch) { /* User-visible flags. */ case '1': @@ -501,9 +503,13 @@ main(int argc, char **argv) setmode(0, O_BINARY); #endif break; + case 'T': + Tflag = 1; + break; default: usage(); } + } argc -= optind; argv += optind; @@ -534,7 +540,7 @@ main(int argc, char **argv) } if (tflag) { /* Receive data. */ - sink(argc, argv); + sink(argc, argv, NULL); exit(errs != 0); } if (argc < 2) @@ -620,6 +626,253 @@ parse_scp_uri(const char *uri, char **userp, char **ho return r; } +/* Appends a string to an array; returns 0 on success, -1 on alloc failure */ +static int +append(char *cp, char ***ap, size_t *np) +{ + char **tmp; + + if ((tmp = reallocarray(*ap, *np + 1, sizeof(*tmp))) == NULL) + return -1; + tmp[(*np)] = cp; + (*np)++; + *ap = tmp; + return 0; +} + +/* + * Finds the start and end of the first brace pair in the pattern. + * returns 0 on success or -1 for invalid patterns. + */ +static int +find_brace(const char *pattern, int *startp, int *endp) +{ + int i; + int in_bracket, brace_level; + + *startp = *endp = -1; + in_bracket = brace_level = 0; + for (i = 0; i < INT_MAX && *endp < 0 && pattern[i] != '\0'; i++) { + switch (pattern[i]) { + case '\\': + /* skip next character */ + if (pattern[i + 1] != '\0') + i++; + break; + case '[': + in_bracket = 1; + break; + case ']': + in_bracket = 0; + break; + case '{': + if (in_bracket) + break; + if (pattern[i + 1] == '}') { + /* Protect a single {}, for find(1), like csh */ + i++; /* skip */ + break; + } + if (*startp == -1) + *startp = i; + brace_level++; + break; + case '}': + if (in_bracket) + break; + if (*startp < 0) { + /* Unbalanced brace */ + return -1; + } + if (--brace_level <= 0) + *endp = i; + break; + } + } + /* unbalanced brackets/braces */ + if (*endp < 0 && (*startp >= 0 || in_bracket)) + return -1; + return 0; +} + +/* + * Assembles and records a successfully-expanded pattern, returns -1 on + * alloc failure. + */ +static int +emit_expansion(const char *pattern, int brace_start, int brace_end, + int sel_start, int sel_end, char ***patternsp, size_t *npatternsp) +{ + char *cp; + int o = 0, tail_len = strlen(pattern + brace_end + 1); + + if ((cp = malloc(brace_start + (sel_end - sel_start) + + tail_len + 1)) == NULL) + return -1; + + /* Pattern before initial brace */ + if (brace_start > 0) { + memcpy(cp, pattern, brace_start); + o = brace_start; + } + /* Current braced selection */ + if (sel_end - sel_start > 0) { + memcpy(cp + o, pattern + sel_start, + sel_end - sel_start); + o += sel_end - sel_start; + } + /* Remainder of pattern after closing brace */ + if (tail_len > 0) { + memcpy(cp + o, pattern + brace_end + 1, tail_len); + o += tail_len; + } + cp[o] = '\0'; + if (append(cp, patternsp, npatternsp) != 0) { + free(cp); + return -1; + } + return 0; +} + +/* + * Expand the first encountered brace in pattern, appending the expanded + * patterns it yielded to the *patternsp array. + * + * Returns 0 on success or -1 on allocation failure. + * + * Signals whether expansion was performed via *expanded and whether + * pattern was invalid via *invalid. + */ +static int +brace_expand_one(const char *pattern, char ***patternsp, size_t *npatternsp, + int *expanded, int *invalid) +{ + int i; + int in_bracket, brace_start, brace_end, brace_level; + int sel_start, sel_end; + + *invalid = *expanded = 0; + + if (find_brace(pattern, &brace_start, &brace_end) != 0) { + *invalid = 1; + return 0; + } else if (brace_start == -1) + return 0; + + in_bracket = brace_level = 0; + for (i = sel_start = brace_start + 1; i < brace_end; i++) { + switch (pattern[i]) { + case '{': + if (in_bracket) + break; + brace_level++; + break; + case '}': + if (in_bracket) + break; + brace_level--; + break; + case '[': + in_bracket = 1; + break; + case ']': + in_bracket = 0; + break; + case '\\': + if (i < brace_end - 1) + i++; /* skip */ + break; + } + if (pattern[i] == ',' || i == brace_end - 1) { + if (in_bracket || brace_level > 0) + continue; + /* End of a selection, emit an expanded pattern */ + + /* Adjust end index for last selection */ + sel_end = (i == brace_end - 1) ? brace_end : i; + if (emit_expansion(pattern, brace_start, brace_end, + sel_start, sel_end, patternsp, npatternsp) != 0) + return -1; + /* move on to the next selection */ + sel_start = i + 1; + continue; + } + } + if (in_bracket || brace_level > 0) { + *invalid = 1; + return 0; + } + /* success */ + *expanded = 1; + return 0; +} + +/* Expand braces from pattern. Returns 0 on success, -1 on failure */ +static int +brace_expand(const char *pattern, char ***patternsp, size_t *npatternsp) +{ + char *cp, *cp2, **active = NULL, **done = NULL; + size_t i, nactive = 0, ndone = 0; + int ret = -1, invalid = 0, expanded = 0; + + *patternsp = NULL; + *npatternsp = 0; + + /* Start the worklist with the original pattern */ + if ((cp = strdup(pattern)) == NULL) + return -1; + if (append(cp, &active, &nactive) != 0) { + free(cp); + return -1; + } + while (nactive > 0) { + cp = active[nactive - 1]; + nactive--; + if (brace_expand_one(cp, &active, &nactive, + &expanded, &invalid) == -1) { + free(cp); + goto fail; + } + if (invalid) + fatal("%s: invalid brace pattern \"%s\"", __func__, cp); + if (expanded) { + /* + * Current entry expanded to new entries on the + * active list; discard the progenitor pattern. + */ + free(cp); + continue; + } + /* + * Pattern did not expand; append the finename component to + * the completed list + */ + if ((cp2 = strrchr(cp, '/')) != NULL) + *cp2++ = '\0'; + else + cp2 = cp; + if (append(xstrdup(cp2), &done, &ndone) != 0) { + free(cp); + goto fail; + } + free(cp); + } + /* success */ + *patternsp = done; + *npatternsp = ndone; + done = NULL; + ndone = 0; + ret = 0; + fail: + for (i = 0; i < nactive; i++) + free(active[i]); + free(active); + for (i = 0; i < ndone; i++) + free(done[i]); + free(done); + return ret; +} + void toremote(int argc, char **argv) { @@ -791,7 +1044,7 @@ tolocal(int argc, char **argv) continue; } free(bp); - sink(1, argv + argc - 1); + sink(1, argv + argc - 1, src); (void) close(remin); remin = remout = -1; } @@ -967,7 +1220,7 @@ rsource(char *name, struct stat *statp) (sizeof(type) != 4 && sizeof(type) != 8)) void -sink(int argc, char **argv) +sink(int argc, char **argv, const char *src) { static BUF buffer; struct stat stb; @@ -983,6 +1236,8 @@ sink(int argc, char **argv) unsigned long long ull; int setimes, targisdir, wrerrno = 0; char ch, *cp, *np, *targ, *why, *vect[1], buf[2048], visbuf[2048]; + char **patterns = NULL; + size_t n, npatterns = 0; struct timeval tv[2]; #define atime tv[0] @@ -1007,10 +1262,18 @@ sink(int argc, char **argv) (void) atomicio(vwrite, remout, "", 1); if (stat(targ, &stb) == 0 && S_ISDIR(stb.st_mode)) targisdir = 1; + if (src != NULL && !iamrecursive && !Tflag) { + /* + * Prepare to try to restrict incoming filenames to match + * the requested destination file glob. + */ + if (brace_expand(src, &patterns, &npatterns) != 0) + fatal("%s: could not expand pattern", __func__); + } for (first = 1;; first = 0) { cp = buf; if (atomicio(read, remin, cp, 1) != 1) - return; + goto done; if (*cp++ == '\n') SCREWUP("unexpected <newline>"); do { @@ -1036,7 +1299,7 @@ sink(int argc, char **argv) } if (buf[0] == 'E') { (void) atomicio(vwrite, remout, "", 1); - return; + goto done; } if (ch == '\n') *--cp = 0; @@ -1106,10 +1369,19 @@ sink(int argc, char **argv) SCREWUP("size out of range"); size = (off_t)ull; - if ((strchr(cp, '/') != NULL) || (strcmp(cp, "..") == 0)) { + if (*cp == '\0' || strchr(cp, '/') != NULL || + strcmp(cp, ".") == 0 || strcmp(cp, "..") == 0) { run_err("error: unexpected filename: %s", cp); exit(1); } + if (npatterns > 0) { + for (n = 0; n < npatterns; n++) { + if (fnmatch(patterns[n], cp, 0) == 0) + break; + } + if (n >= npatterns) + SCREWUP("filename does not match request"); + } if (targisdir) { static char *namebuf; static size_t cursize; @@ -1147,7 +1419,7 @@ sink(int argc, char **argv) goto bad; } vect[0] = xstrdup(np); - sink(1, vect); + sink(1, vect, src); if (setimes) { setimes = 0; if (utimes(vect[0], tv) < 0) @@ -1268,7 +1540,15 @@ bad: run_err("%s: %s", np, strerror(errno)); break; } } +done: + for (n = 0; n < npatterns; n++) + free(patterns[n]); + free(patterns); + return; screwup: + for (n = 0; n < npatterns; n++) + free(patterns[n]); + free(patterns); run_err("protocol error: %s", why); exit(1); } @@ -1315,7 +1595,7 @@ void usage(void) { (void) fprintf(stderr, - "usage: scp [-346BCpqrv] [-c cipher] [-F ssh_config] [-i identity_file]\n" + "usage: scp [-346BCpqrTv] [-c cipher] [-F ssh_config] [-i identity_file]\n" " [-l limit] [-o ssh_option] [-P port] [-S program] source ... target\n"); exit(1); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201905142254.x4EMsH6O047905>