Sanitize scp filenames via snmprintf. To do this we move the

progressmeter formatting outside of signal handler context and have
the atomicio callback called for EINTR too.  bz#2434 with contributions
from djm and jjelen at redhat.com, ok djm@

5 files changed, 55 insertions, 44 deletions

diff --git a/usr.bin/ssh/atomicio.c b/usr.bin/ssh/atomicio.c
index 58916384919..c922d7c69a7 100644
--- a/usr.bin/ssh/atomicio.c
+++ b/usr.bin/ssh/atomicio.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: atomicio.c,v 1.28 2016/07/27 23:18:12 djm Exp $ */
+/* $OpenBSD: atomicio.c,v 1.29 2019/01/23 08:01:46 dtucker Exp $ */
 /*
  * Copyright (c) 2006 Damien Miller. All rights reserved.
  * Copyright (c) 2005 Anil Madhavapeddy. All rights reserved.
@@ -54,9 +54,15 @@ atomicio6(ssize_t (*f) (int, void *, size_t), int fd, void *_s, size_t n,
 		res = (f) (fd, s + pos, n - pos);
 		switch (res) {
 		case -1:
-			if (errno == EINTR)
+			switch(errno) {
+			case EINTR:
+				/* possible SIGALARM, update callback */
+				if (cb != NULL && cb(cb_arg, 0) == -1) {
+					errno = EINTR;
+					return pos;
+				}
 				continue;
-			if (errno == EAGAIN) {
+			case EAGAIN:
 				(void)poll(&pfd, 1, -1);
 				continue;
 			}
@@ -107,9 +113,15 @@ atomiciov6(ssize_t (*f) (int, const struct iovec *, int), int fd,
 		res = (f) (fd, iov, iovcnt);
 		switch (res) {
 		case -1:
-			if (errno == EINTR)
+			switch(errno) {
+			case EINTR:
+				/* possible SIGALARM, update callback */
+				if (cb != NULL && cb(cb_arg, 0) == -1) {
+					errno = EINTR;
+					return pos;
+				}
 				continue;
-			if (errno == EAGAIN) {
+			case EAGAIN:
 				(void)poll(&pfd, 1, -1);
 				continue;
 			}
diff --git a/usr.bin/ssh/progressmeter.c b/usr.bin/ssh/progressmeter.c
index 3220e2cf4e4..d440e4b61f9 100644
--- a/usr.bin/ssh/progressmeter.c
+++ b/usr.bin/ssh/progressmeter.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: progressmeter.c,v 1.45 2016/06/30 05:17:05 dtucker Exp $ */
+/* $OpenBSD: progressmeter.c,v 1.46 2019/01/23 08:01:46 dtucker Exp $ */
 /*
  * Copyright (c) 2003 Nils Nordman.  All rights reserved.
  *
@@ -29,6 +29,7 @@
 
 #include <errno.h>
 #include <signal.h>
+#include <stdarg.h>
 #include <stdio.h>
 #include <string.h>
 #include <time.h>
@@ -37,6 +38,7 @@
 #include "progressmeter.h"
 #include "atomicio.h"
 #include "misc.h"
+#include "utf8.h"
 
 #define DEFAULT_WINSIZE 80
 #define MAX_WINSIZE 512
@@ -59,7 +61,7 @@ static void setscreensize(void);
 void refresh_progress_meter(void);
 
 /* signal handler for updating the progress meter */
-static void update_progress_meter(int);
+static void sig_alarm(int);
 
 static double start;		/* start progress */
 static double last_update;	/* last progress update */
@@ -72,6 +74,7 @@ static long stalled;		/* how long we have been stalled */
 static int bytes_per_second;	/* current speed in bytes per second */
 static int win_size;		/* terminal window size */
 static volatile sig_atomic_t win_resized; /* for window resizing */
+static volatile sig_atomic_t alarm_fired;
 
 /* units for format_size */
 static const char unit[] = " KMGT";
@@ -124,9 +127,17 @@ refresh_progress_meter(void)
 	off_t bytes_left;
 	int cur_speed;
 	int hours, minutes, seconds;
-	int i, len;
 	int file_len;
 
+	if ((!alarm_fired && !win_resized) || !can_output())
+		return;
+	alarm_fired = 0;
+
+	if (win_resized) {
+		setscreensize();
+		win_resized = 0;
+	}
+
 	transferred = *counter - (cur_pos ? cur_pos : start_pos);
 	cur_pos = *counter;
 	now = monotime_double();
@@ -156,16 +167,11 @@ refresh_progress_meter(void)
 
 	/* filename */
 	buf[0] = '\0';
-	file_len = win_size - 35;
+	file_len = win_size - 36;
 	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';
+		buf[0] = '\r';
+		snmprintf(buf+1, sizeof(buf)-1 , &file_len, "%*s",
+		    file_len * -1, file);
 	}
 
 	/* percent of transfer done */
@@ -226,22 +232,11 @@ refresh_progress_meter(void)
 
 /*ARGSUSED*/
 static void
-update_progress_meter(int ignore)
+sig_alarm(int ignore)
 {
-	int save_errno;
-
-	save_errno = errno;
-
-	if (win_resized) {
-		setscreensize();
-		win_resized = 0;
-	}
-	if (can_output())
-		refresh_progress_meter();
-
-	signal(SIGALRM, update_progress_meter);
+	signal(SIGALRM, sig_alarm);
+	alarm_fired = 1;
 	alarm(UPDATE_INTERVAL);
-	errno = save_errno;
 }
 
 void
@@ -257,10 +252,9 @@ start_progress_meter(const char *f, off_t filesize, off_t *ctr)
 	bytes_per_second = 0;
 
 	setscreensize();
-	if (can_output())
-		refresh_progress_meter();
+	refresh_progress_meter();
 
-	signal(SIGALRM, update_progress_meter);
+	signal(SIGALRM, sig_alarm);
 	signal(SIGWINCH, sig_winch);
 	alarm(UPDATE_INTERVAL);
 }
@@ -284,6 +278,7 @@ stop_progress_meter(void)
 static void
 sig_winch(int sig)
 {
+	signal(SIGWINCH, sig_winch);
 	win_resized = 1;
 }
 
diff --git a/usr.bin/ssh/progressmeter.h b/usr.bin/ssh/progressmeter.h
index bf179dca651..8f667806019 100644
--- a/usr.bin/ssh/progressmeter.h
+++ b/usr.bin/ssh/progressmeter.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: progressmeter.h,v 1.3 2015/01/14 13:54:13 djm Exp $ */
+/* $OpenBSD: progressmeter.h,v 1.4 2019/01/23 08:01:46 dtucker Exp $ */
 /*
  * Copyright (c) 2002 Nils Nordman.  All rights reserved.
  *
@@ -24,4 +24,5 @@
  */
 
 void	start_progress_meter(const char *, off_t, off_t *);
+void	refresh_progress_meter(void);
 void	stop_progress_meter(void);
diff --git a/usr.bin/ssh/scp.c b/usr.bin/ssh/scp.c
index 44f064d0bbc..a87428bdc2a 100644
--- a/usr.bin/ssh/scp.c
+++ b/usr.bin/ssh/scp.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: scp.c,v 1.199 2019/01/21 22:50:42 tb Exp $ */
+/* $OpenBSD: scp.c,v 1.200 2019/01/23 08:01:46 dtucker Exp $ */
 /*
  * scp - secure remote copy.  This is basically patched BSD rcp which
  * uses ssh to do the data transfer (instead of using rcmd).
@@ -563,6 +563,7 @@ scpio(void *_cnt, size_t s)
 	off_t *cnt = (off_t *)_cnt;
 
 	*cnt += s;
+	refresh_progress_meter();
 	if (limit_kbps > 0)
 		bandwidth_limit(&bwlimit, s);
 	return 0;
diff --git a/usr.bin/ssh/sftp-client.c b/usr.bin/ssh/sftp-client.c
index d6dd33bf6b8..e71a9408a86 100644
--- a/usr.bin/ssh/sftp-client.c
+++ b/usr.bin/ssh/sftp-client.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sftp-client.c,v 1.131 2019/01/16 23:23:45 djm Exp $ */
+/* $OpenBSD: sftp-client.c,v 1.132 2019/01/23 08:01:46 dtucker Exp $ */
 /*
  * Copyright (c) 2001-2004 Damien Miller <djm@openbsd.org>
  *
@@ -88,7 +88,9 @@ sftpio(void *_bwlimit, size_t amount)
 {
 	struct bwlimit *bwlimit = (struct bwlimit *)_bwlimit;
 
-	bandwidth_limit(bwlimit, amount);
+	refresh_progress_meter();
+	if (bwlimit != NULL)
+		bandwidth_limit(bwlimit, amount);
 	return 0;
 }
 
@@ -108,8 +110,8 @@ send_msg(struct sftp_conn *conn, struct sshbuf *m)
 	iov[1].iov_base = (u_char *)sshbuf_ptr(m);
 	iov[1].iov_len = sshbuf_len(m);
 
-	if (atomiciov6(writev, conn->fd_out, iov, 2,
-	    conn->limit_kbps > 0 ? sftpio : NULL, &conn->bwlimit_out) !=
+	if (atomiciov6(writev, conn->fd_out, iov, 2, sftpio,
+	    conn->limit_kbps > 0 ? &conn->bwlimit_out : NULL) !=
 	    sshbuf_len(m) + sizeof(mlen))
 		fatal("Couldn't send packet: %s", strerror(errno));
 
@@ -125,8 +127,8 @@ get_msg_extended(struct sftp_conn *conn, struct sshbuf *m, int initial)
 
 	if ((r = sshbuf_reserve(m, 4, &p)) != 0)
 		fatal("%s: buffer error: %s", __func__, ssh_err(r));
-	if (atomicio6(read, conn->fd_in, p, 4,
-	    conn->limit_kbps > 0 ? sftpio : NULL, &conn->bwlimit_in) != 4) {
+	if (atomicio6(read, conn->fd_in, p, 4, sftpio,
+	    conn->limit_kbps > 0 ? &conn->bwlimit_in : NULL) != 4) {
 		if (errno == EPIPE || errno == ECONNRESET)
 			fatal("Connection closed");
 		else
@@ -144,8 +146,8 @@ get_msg_extended(struct sftp_conn *conn, struct sshbuf *m, int initial)
 
 	if ((r = sshbuf_reserve(m, msg_len, &p)) != 0)
 		fatal("%s: buffer error: %s", __func__, ssh_err(r));
-	if (atomicio6(read, conn->fd_in, p, msg_len,
-	    conn->limit_kbps > 0 ? sftpio : NULL, &conn->bwlimit_in)
+	if (atomicio6(read, conn->fd_in, p, msg_len, sftpio,
+	    conn->limit_kbps > 0 ? &conn->bwlimit_in : NULL)
 	    != msg_len) {
 		if (errno == EPIPE)
 			fatal("Connection closed");