Fix insufficient filename validation in scp client

Approved by:	so
Security:	FreeBSD-EN-19:10.scp

2 files changed, 304 insertions, 14 deletions

diff --git a/crypto/openssh/scp.1 b/crypto/openssh/scp.1
index 92abcaf075a0..4dca60035bd3 100644
--- a/crypto/openssh/scp.1
+++ b/crypto/openssh/scp.1
@@ -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
diff --git a/crypto/openssh/scp.c b/crypto/openssh/scp.c
index 60682c68769b..cbf67b69de5e 100644
--- a/crypto/openssh/scp.c
+++ b/crypto/openssh/scp.c
@@ -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 **hostp, int *portp,
 	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);
 }