From 955854cafca88e0cdcd3d09ca1ad4ada465364a1 Mon Sep 17 00:00:00 2001
From: "djm@openbsd.org" <djm@openbsd.org>
Date: Wed, 6 May 2020 20:57:38 +0000
Subject: [PATCH] upstream: another case where a utimes() failure could make
 scp send

a desynchronising error; reminded by Aymeric Vincent ok deraadt markus

OpenBSD-Commit-ID: 2ea611d34d8ff6d703a7a8bf858aa5dbfbfa7381
---

From aad87b88fc2536b1ea023213729aaf4eaabe1894 Mon Sep 17 00:00:00 2001
From: "djm@openbsd.org" <djm@openbsd.org>
Date: Fri, 1 May 2020 06:31:42 +0000
Subject: [PATCH] upstream: when receving a file in sink(), be careful to send
 at

most a single error response after the file has been opened. Otherwise the
source() and sink() can become desyncronised. Reported by Daniel Goujot,
Georges-Axel Jaloyan, Ryan Lahfa, and David Naccache.

ok deraadt@ markus@

OpenBSD-Commit-ID: 6c14d233c97349cb811a8f7921ded3ae7d9e0035

---
From 31909696c4620c431dd55f6cd15db65c4e9b98da Mon Sep 17 00:00:00 2001
From: "djm@openbsd.org" <djm@openbsd.org>
Date: Fri, 1 May 2020 06:28:52 +0000
Subject: [PATCH] upstream: expose vasnmprintf(); ok (as part of other commit)
 markus

deraadt

OpenBSD-Commit-ID: 2e80cea441c599631a870fd40307d2ade5a7f9b5
---

 scp.c  |   99 +++++++++++++++++++++++++++++++++++++++--------------------------
 utf8.c |    3 -
 utf8.h |    1 
 3 files changed, 62 insertions(+), 41 deletions(-)

--- a/scp.c
+++ b/scp.c
@@ -114,6 +114,7 @@
 #include "log.h"
 #include "misc.h"
 #include "progressmeter.h"
+#include "utf8.h"
 
 extern char *__progname;
 
@@ -348,6 +349,7 @@ BUF *allocbuf(BUF *, int, int);
 void lostconn(int);
 int okname(char *);
 void run_err(const char *,...);
+int note_err(const char *,...);
 void verifydir(char *);
 
 struct passwd *pwd;
@@ -1157,16 +1159,13 @@ sink(int argc, char **argv, const char *
 {
 	static BUF buffer;
 	struct stat stb;
-	enum {
-		YES, NO, DISPLAYED
-	} wrerr;
 	BUF *bp;
 	off_t i;
 	size_t j, count;
 	int amt, exists, first, ofd;
 	mode_t mode, omode, mask;
 	off_t size, statbytes;
-	int setimes, targisdir, wrerrno = 0;
+	int setimes, targisdir, wrerr;
 	char ch, *cp, *np, *targ, *why, *vect[1], buf[2048];
 	char **patterns = NULL;
 	size_t n, npatterns = 0;
@@ -1330,9 +1329,7 @@ sink(int argc, char **argv, const char *
 			sink(1, vect, src);
 			if (setimes) {
 				setimes = 0;
-				if (utimes(vect[0], tv) < 0)
-					run_err("%s: set times: %s",
-					    vect[0], strerror(errno));
+				(void) utimes(vect[0], tv);
 			}
 			if (mod_flag)
 				(void) chmod(vect[0], mode);
@@ -1352,8 +1349,13 @@ bad:			run_err("%s: %s", np, strerror(er
 			continue;
 		}
 		cp = bp->buf;
-		wrerr = NO;
+		wrerr = 0;
 
+		/*
+		 * NB. do not use run_err() unless immediately followed by
+		 * exit() below as it may send a spurious reply that might
+		 * desyncronise us from the peer. Use note_err() instead.
+		 */
 		statbytes = 0;
 		if (showprogress)
 			start_progress_meter(curfile, size, &statbytes);
@@ -1378,11 +1380,12 @@ bad:			run_err("%s: %s", np, strerror(er
 
 			if (count == bp->cnt) {
 				/* Keep reading so we stay sync'd up. */
-				if (wrerr == NO) {
+				if (!wrerr) {
 					if (atomicio(vwrite, ofd, bp->buf,
 					    count) != count) {
-						wrerr = YES;
-						wrerrno = errno;
+						note_err("%s: %s", np,
+						    strerror(errno));
+						wrerr = 1;
 					}
 				}
 				count = 0;
@@ -1392,16 +1395,14 @@ bad:			run_err("%s: %s", np, strerror(er
 		unset_nonblock(remin);
 		if (showprogress)
 			stop_progress_meter();
-		if (count != 0 && wrerr == NO &&
+		if (count != 0 && !wrerr &&
 		    atomicio(vwrite, ofd, bp->buf, count) != count) {
-			wrerr = YES;
-			wrerrno = errno;
-		}
-		if (wrerr == NO && (!exists || S_ISREG(stb.st_mode)) &&
-		    ftruncate(ofd, size) != 0) {
-			run_err("%s: truncate: %s", np, strerror(errno));
-			wrerr = DISPLAYED;
+		note_err("%s: %s", np, strerror(errno));
+			wrerr = 1;
 		}
+		if (!wrerr && (!exists || S_ISREG(stb.st_mode)) &&
+		    ftruncate(ofd, size) != 0)
+			note_err("%s: truncate: %s", np, strerror(errno));
 		if (pflag) {
 			if (exists || omode != mode)
 #ifdef HAVE_FCHMOD
@@ -1409,9 +1410,8 @@ bad:			run_err("%s: %s", np, strerror(er
 #else /* HAVE_FCHMOD */
 				if (chmod(np, omode)) {
 #endif /* HAVE_FCHMOD */
-					run_err("%s: set mode: %s",
+					note_err("%s: set mode: %s",
 					    np, strerror(errno));
-					wrerr = DISPLAYED;
 				}
 		} else {
 			if (!exists && omode != mode)
@@ -1420,34 +1420,23 @@ bad:			run_err("%s: %s", np, strerror(er
 #else /* HAVE_FCHMOD */
 				if (chmod(np, omode & ~mask)) {
 #endif /* HAVE_FCHMOD */
-					run_err("%s: set mode: %s",
+					note_err("%s: set mode: %s",
 					    np, strerror(errno));
-					wrerr = DISPLAYED;
 				}
 		}
-		if (close(ofd) == -1) {
-			wrerr = YES;
-			wrerrno = errno;
-		}
+		if (close(ofd) == -1)
+			note_err(np, "%s: close: %s", np, strerror(errno));
 		(void) response();
-		if (setimes && wrerr == NO) {
+		if (setimes && !wrerr) {
 			setimes = 0;
 			if (utimes(np, tv) < 0) {
-				run_err("%s: set times: %s",
+				note_err("%s: set times: %s",
 				    np, strerror(errno));
-				wrerr = DISPLAYED;
 			}
 		}
-		switch (wrerr) {
-		case YES:
-			run_err("%s: %s", np, strerror(wrerrno));
-			break;
-		case NO:
+		/* If no error was noted then signal success for this file */
+		if (note_err(NULL) == 0)
 			(void) atomicio(vwrite, remout, "", 1);
-			break;
-		case DISPLAYED:
-			break;
-		}
 	}
 done:
 	for (n = 0; n < npatterns; n++)
@@ -1530,6 +1519,38 @@ run_err(const char *fmt,...)
 	}
 }
 
+/*
+ * Notes a sink error for sending at the end of a file transfer. Returns 0 if
+ * no error has been noted or -1 otherwise. Use note_err(NULL) to flush
+ * any active error at the end of the transfer.
+ */
+int
+note_err(const char *fmt, ...)
+{
+	static char *emsg;
+	va_list ap;
+
+	/* Replay any previously-noted error */
+	if (fmt == NULL) {
+		if (emsg == NULL)
+			return 0;
+		run_err("%s", emsg);
+		free(emsg);
+		emsg = NULL;
+		return -1;
+	}
+
+	errs++;
+	/* Prefer first-noted error */
+	if (emsg != NULL)
+		return -1;
+
+	va_start(ap, fmt);
+	vasnmprintf(&emsg, INT_MAX, NULL, fmt, ap);
+	va_end(ap);
+	return -1;
+}
+
 void
 verifydir(char *cp)
 {
--- a/utf8.c
+++ b/utf8.c
@@ -38,7 +38,6 @@
 
 static int	 dangerous_locale(void);
 static int	 grow_dst(char **, size_t *, size_t, char **, size_t);
-static int	 vasnmprintf(char **, size_t, int *, const char *, va_list);
 
 
 /*
@@ -89,7 +88,7 @@ grow_dst(char **dst, size_t *sz, size_t
  * written is returned in *wp.
  */
 
-static int
+int
 vasnmprintf(char **str, size_t maxsz, int *wp, const char *fmt, va_list ap)
 {
 	char	*src;	/* Source string returned from vasprintf. */
--- a/utf8.h
+++ b/utf8.h
@@ -15,5 +15,6 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+int	 vasnmprintf(char **, size_t, int *, const char *, va_list);
 int	 snmprintf(char *, size_t, int *, const char *, ...)
 	     __attribute__((format(printf, 4, 5)));
