upstream commit

To prevent screwing up terminal settings when printing to
 the terminal, for ASCII and UTF-8, escape bytes not forming characters and
 bytes forming non-printable characters with vis(3) VIS_OCTAL. For other
 character sets, abort printing of the current string in these cases.  In
 particular, * let scp(1) respect the local user's LC_CTYPE locale(1); *
 sanitize data received from the remote host; * sanitize filenames, usernames,
 and similar data even locally; * take character display widths into account
 for the progressmeter.

This is believed to be sufficient to keep the local terminal safe
on OpenBSD, but bad things can still happen on other systems with
state-dependent locales because many places in the code print
unencoded ASCII characters into the output stream.

Using feedback from djm@ and martijn@,
various aspects discussed with many others.

deraadt@ says it should go in now, i probably already hesitated too long

Upstream-ID: e66afbc94ee396ddcaffd433b9a3b80f387647e0

1 files changed, 28 insertions, 17 deletions

diff --git a/scp.c b/scp.c
index 3f0d7509..43ca3fa0 100644
--- a/scp.c
+++ b/scp.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: scp.c,v 1.185 2016/03/02 22:43:52 dtucker Exp $ */
+/* $OpenBSD: scp.c,v 1.186 2016/05/25 23:48:45 schwarze Exp $ */
 /*
  * scp - secure remote copy.  This is basically patched BSD rcp which
  * uses ssh to do the data transfer (instead of using rcmd).
@@ -96,6 +96,7 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <limits.h>
+#include <locale.h>
 #include <pwd.h>
 #include <signal.h>
 #include <stdarg.h>
@@ -114,6 +115,7 @@
 #include "log.h"
 #include "misc.h"
 #include "progressmeter.h"
+#include "utf8.h"
 
 extern char *__progname;
 
@@ -191,7 +193,7 @@ do_local_cmd(arglist *a)
 	if (verbose_mode) {
 		fprintf(stderr, "Executing:");
 		for (i = 0; i < a->num; i++)
-			fprintf(stderr, " %s", a->list[i]);
+			fmprintf(stderr, " %s", a->list[i]);
 		fprintf(stderr, "\n");
 	}
 	if ((pid = fork()) == -1)
@@ -232,7 +234,7 @@ do_cmd(char *host, char *remuser, char *cmd, int *fdin, int *fdout)
 	int pin[2], pout[2], reserved[2];
 
 	if (verbose_mode)
-		fprintf(stderr,
+		fmprintf(stderr,
 		    "Executing: program %s host %s, user %s, command %s\n",
 		    ssh_program, host,
 		    remuser ? remuser : "(unspecified)", cmd);
@@ -307,7 +309,7 @@ do_cmd2(char *host, char *remuser, char *cmd, int fdin, int fdout)
 	int status;
 
 	if (verbose_mode)
-		fprintf(stderr,
+		fmprintf(stderr,
 		    "Executing: 2nd program %s host %s, user %s, command %s\n",
 		    ssh_program, host,
 		    remuser ? remuser : "(unspecified)", cmd);
@@ -378,6 +380,8 @@ main(int argc, char **argv)
 	/* Ensure that fds 0, 1 and 2 are open or directed to /dev/null */
 	sanitise_stdfd();
 
+	setlocale(LC_CTYPE, "");
+
 	/* Copy argv, because we modify it */
 	newargv = xcalloc(MAX(argc + 1, 1), sizeof(*newargv));
 	for (n = 0; n < argc; n++)
@@ -810,9 +814,8 @@ syserr:			run_err("%s: %s", name, strerror(errno));
 		snprintf(buf, sizeof buf, "C%04o %lld %s\n",
 		    (u_int) (stb.st_mode & FILEMODEMASK),
 		    (long long)stb.st_size, last);
-		if (verbose_mode) {
-			fprintf(stderr, "Sending file modes: %s", buf);
-		}
+		if (verbose_mode)
+			fmprintf(stderr, "Sending file modes: %s", buf);
 		(void) atomicio(vwrite, remout, buf, strlen(buf));
 		if (response() < 0)
 			goto next;
@@ -889,7 +892,7 @@ rsource(char *name, struct stat *statp)
 	(void) snprintf(path, sizeof path, "D%04o %d %.1024s\n",
 	    (u_int) (statp->st_mode & FILEMODEMASK), 0, last);
 	if (verbose_mode)
-		fprintf(stderr, "Entering directory: %s", path);
+		fmprintf(stderr, "Entering directory: %s", path);
 	(void) atomicio(vwrite, remout, path, strlen(path));
 	if (response() < 0) {
 		closedir(dirp);
@@ -929,7 +932,7 @@ sink(int argc, char **argv)
 	off_t size, statbytes;
 	unsigned long long ull;
 	int setimes, targisdir, wrerrno = 0;
-	char ch, *cp, *np, *targ, *why, *vect[1], buf[2048];
+	char ch, *cp, *np, *targ, *why, *vect[1], buf[2048], visbuf[2048];
 	struct timeval tv[2];
 
 #define	atime	tv[0]
@@ -964,12 +967,15 @@ sink(int argc, char **argv)
 		} while (cp < &buf[sizeof(buf) - 1] && ch != '\n');
 		*cp = 0;
 		if (verbose_mode)
-			fprintf(stderr, "Sink: %s", buf);
+			fmprintf(stderr, "Sink: %s", buf);
 
 		if (buf[0] == '\01' || buf[0] == '\02') {
-			if (iamremote == 0)
+			if (iamremote == 0) {
+				(void) snmprintf(visbuf, sizeof(visbuf),
+				    NULL, "%s", buf + 1);
 				(void) atomicio(vwrite, STDERR_FILENO,
-				    buf + 1, strlen(buf + 1));
+				    visbuf, strlen(visbuf));
+			}
 			if (buf[0] == '\02')
 				exit(1);
 			++errs;
@@ -1212,7 +1218,7 @@ screwup:
 int
 response(void)
 {
-	char ch, *cp, resp, rbuf[2048];
+	char ch, *cp, resp, rbuf[2048], visbuf[2048];
 
 	if (atomicio(read, remin, &resp, sizeof(resp)) != sizeof(resp))
 		lostconn(0);
@@ -1232,8 +1238,13 @@ response(void)
 			*cp++ = ch;
 		} while (cp < &rbuf[sizeof(rbuf) - 1] && ch != '\n');
 
-		if (!iamremote)
-			(void) atomicio(vwrite, STDERR_FILENO, rbuf, cp - rbuf);
+		if (!iamremote) {
+			cp[-1] = '\0';
+			(void) snmprintf(visbuf, sizeof(visbuf),
+			    NULL, "%s\n", rbuf);
+			(void) atomicio(vwrite, STDERR_FILENO,
+			    visbuf, strlen(visbuf));
+		}
 		++errs;
 		if (resp == 1)
 			return (-1);
@@ -1271,7 +1282,7 @@ run_err(const char *fmt,...)
 
 	if (!iamremote) {
 		va_start(ap, fmt);
-		vfprintf(stderr, fmt, ap);
+		vfmprintf(stderr, fmt, ap);
 		va_end(ap);
 		fprintf(stderr, "\n");
 	}
@@ -1317,7 +1328,7 @@ okname(char *cp0)
 	} while (*++cp);
 	return (1);
 
-bad:	fprintf(stderr, "%s: invalid user name\n", cp0);
+bad:	fmprintf(stderr, "%s: invalid user name\n", cp0);
 	return (0);
 }