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, 27 insertions, 24 deletions

diff --git a/progressmeter.c b/progressmeter.c
index 3a455408..4fed2f4f 100644
--- a/progressmeter.c
+++ b/progressmeter.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: progressmeter.c,v 1.42 2016/03/02 22:42:40 dtucker Exp $ */
+/* $OpenBSD: progressmeter.c,v 1.43 2016/05/25 23:48:45 schwarze Exp $ */
 /*
  * Copyright (c) 2003 Nils Nordman.  All rights reserved.
  *
@@ -31,6 +31,7 @@
 
 #include <errno.h>
 #include <signal.h>
+#include <stdarg.h>
 #include <stdio.h>
 #include <string.h>
 #include <time.h>
@@ -39,6 +40,7 @@
 #include "progressmeter.h"
 #include "atomicio.h"
 #include "misc.h"
+#include "utf8.h"
 
 #define DEFAULT_WINSIZE 80
 #define MAX_WINSIZE 512
@@ -119,14 +121,14 @@ format_size(char *buf, int size, off_t bytes)
 void
 refresh_progress_meter(void)
 {
-	char buf[MAX_WINSIZE + 1];
+	char buf[MAX_WINSIZE * 4 + 1];
 	off_t transferred;
 	double elapsed, now;
 	int percent;
 	off_t bytes_left;
 	int cur_speed;
 	int hours, minutes, seconds;
-	int i, len;
+	size_t i;
 	int file_len;
 
 	transferred = *counter - (cur_pos ? cur_pos : start_pos);
@@ -157,17 +159,16 @@ refresh_progress_meter(void)
 		bytes_per_second = cur_speed;
 
 	/* filename */
-	buf[0] = '\0';
+	buf[0] = '\r';
+	buf[1] = '\0';
 	file_len = win_size - 35;
 	if (file_len > 0) {
-		len = snprintf(buf, file_len + 1, "\r%s", file);
-		if (len < 0)
-			len = 0;
-		if (len >= file_len + 1)
-			len = file_len;
-		for (i = len; i < file_len; i++)
-			buf[i] = ' ';
-		buf[file_len] = '\0';
+		(void) snmprintf(buf + 1, sizeof(buf) - 1 - 35,
+		    &file_len, "%s", file);
+		i = strlen(buf);
+		while (++file_len < win_size - 35 && i + 1 < sizeof(buf))
+			buf[i++] = ' ';
+		buf[i] = '\0';
 	}
 
 	/* percent of transfer done */
@@ -175,18 +176,18 @@ refresh_progress_meter(void)
 		percent = ((float)cur_pos / end_pos) * 100;
 	else
 		percent = 100;
-	snprintf(buf + strlen(buf), win_size - strlen(buf),
+	snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
 	    " %3d%% ", percent);
 
 	/* amount transferred */
-	format_size(buf + strlen(buf), win_size - strlen(buf),
+	format_size(buf + strlen(buf), sizeof(buf) - strlen(buf),
 	    cur_pos);
-	strlcat(buf, " ", win_size);
+	strlcat(buf, " ", sizeof(buf));
 
 	/* bandwidth usage */
-	format_rate(buf + strlen(buf), win_size - strlen(buf),
+	format_rate(buf + strlen(buf), sizeof(buf) - strlen(buf),
 	    (off_t)bytes_per_second);
-	strlcat(buf, "/s ", win_size);
+	strlcat(buf, "/s ", sizeof(buf));
 
 	/* ETA */
 	if (!transferred)
@@ -195,9 +196,9 @@ refresh_progress_meter(void)
 		stalled = 0;
 
 	if (stalled >= STALL_TIME)
-		strlcat(buf, "- stalled -", win_size);
+		strlcat(buf, "- stalled -", sizeof(buf));
 	else if (bytes_per_second == 0 && bytes_left)
-		strlcat(buf, "  --:-- ETA", win_size);
+		strlcat(buf, "  --:-- ETA", sizeof(buf));
 	else {
 		if (bytes_left > 0)
 			seconds = bytes_left / bytes_per_second;
@@ -210,19 +211,21 @@ refresh_progress_meter(void)
 		seconds -= minutes * 60;
 
 		if (hours != 0)
-			snprintf(buf + strlen(buf), win_size - strlen(buf),
+			snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
 			    "%d:%02d:%02d", hours, minutes, seconds);
 		else
-			snprintf(buf + strlen(buf), win_size - strlen(buf),
+			snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
 			    "  %02d:%02d", minutes, seconds);
 
 		if (bytes_left > 0)
-			strlcat(buf, " ETA", win_size);
+			strlcat(buf, " ETA", sizeof(buf));
 		else
-			strlcat(buf, "    ", win_size);
+			strlcat(buf, "    ", sizeof(buf));
 	}
+	if (win_size < 35)
+		buf[win_size] = '\0';
 
-	atomicio(vwrite, STDOUT_FILENO, buf, win_size - 1);
+	atomicio(vwrite, STDOUT_FILENO, buf, strlen(buf));
 	last_update = now;
 }