File openssh-7.9p1-scp-name-validator.patch of Package openssh

Overview Repositories Revisions Requests Users Attributes Meta

File openssh-7.9p1-scp-name-validator.patch of Package openssh (Revision 92aab3767077f1ec9aa27e013729a787)

Currently displaying revision 92aab3767077f1ec9aa27e013729a787 , Show latest

diff --git a/defines.h b/defines.h
index 8f421306..8b4af9b2 100644
--- a/defines.h
+++ b/defines.h
@@ -873,4 +873,10 @@ struct winsize {
 # define USE_SYSTEM_GLOB
 #endif
 
+/*
+ * Define to enable additional scp file name validation against
+ * malicious servers.
+ */
+#define USE_SCP_NAME_VALIDATOR 1
+
 #endif /* _DEFINES_H */
diff --git a/progressmeter.c b/progressmeter.c
index fe9bf52e..b2a3a38d 100644
--- a/progressmeter.c
+++ b/progressmeter.c
@@ -160,7 +160,10 @@ refresh_progress_meter(void)
 	buf[0] = '\0';
 	file_len = win_size - 35;
 	if (file_len > 0) {
-		len = snprintf(buf, file_len + 1, "\r%s", file);
+		char visbuf[MAX_WINSIZE + 1];
+		(void) snmprintf(visbuf, sizeof(visbuf),
+		                 NULL, "%s", file);
+		len = snprintf(buf, file_len + 1, "\r%s", visbuf);
 		if (len < 0)
 			len = 0;
 		if (len >= file_len + 1)
diff --git a/regress/scp-ssh-wrapper.sh b/regress/scp-ssh-wrapper.sh
index 59f1ff63..dd48a482 100644
--- a/regress/scp-ssh-wrapper.sh
+++ b/regress/scp-ssh-wrapper.sh
@@ -51,6 +51,18 @@ badserver_4)
 	echo "C755 2 file"
 	echo "X"
 	;;
+badserver_5)
+	echo "D0555 0 "
+	echo "X"
+	;;
+badserver_6)
+	echo "D0555 0 ."
+	echo "X"
+	;;
+badserver_7)
+	echo "C0755 2 extrafile"
+	echo "X"
+	;;
 *)
 	set -- $arg
 	shift
diff --git a/regress/scp.sh b/regress/scp.sh
index 57cc7706..104c89e1 100644
--- a/regress/scp.sh
+++ b/regress/scp.sh
@@ -25,6 +25,7 @@ export SCP # used in scp-ssh-wrapper.scp
 scpclean() {
 	rm -rf ${COPY} ${COPY2} ${DIR} ${DIR2}
 	mkdir ${DIR} ${DIR2}
+	chmod 755 ${DIR} ${DIR2}
 }
 
 verbose "$tid: simple copy local file to local file"
@@ -101,7 +102,7 @@ if [ ! -z "$SUDO" ]; then
 	$SUDO rm ${DIR2}/copy
 fi
 
-for i in 0 1 2 3 4; do
+for i in 0 1 2 3 4 5 6 7; do
 	verbose "$tid: disallow bad server #$i"
 	SCPTESTMODE=badserver_$i
 	export DIR SCPTESTMODE
@@ -113,6 +114,15 @@ for i in 0 1 2 3 4; do
 	scpclean
 	$SCP -r $scpopts somehost:${DATA} ${DIR2} >/dev/null 2>/dev/null
 	[ -d ${DIR}/dotpathdir ] && fail "allows dir creation outside of subdir"
+
+	scpclean
+	$SCP -pr $scpopts somehost:${DATA} ${DIR2} >/dev/null 2>/dev/null
+	[ ! -w ${DIR2} ] && fail "allows target root attribute change"
+
+	scpclean
+	$SCP $scpopts somehost:${DATA} ${DIR2} >/dev/null 2>/dev/null
+	[ -e ${DIR2}/extrafile ] && fail "allows extranous object creation"
+	rm -f ${DIR2}/extrafile
 done
 
 verbose "$tid: detect non-directory target"
diff --git a/scp.c b/scp.c
index eb17c341..da1a3a44 100644
--- a/scp.c
+++ b/scp.c
@@ -17,6 +17,7 @@
 /*
  * Copyright (c) 1999 Theo de Raadt.  All rights reserved.
  * Copyright (c) 1999 Aaron Campbell.  All rights reserved.
+ * Copyright (c) 2019 Harry Sintonen.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -87,6 +88,14 @@
 #ifdef HAVE_SYS_TIME_H
 # include <sys/time.h>
 #endif
+#ifdef USE_SCP_NAME_VALIDATOR
+# include <libgen.h>
+# ifdef USE_SYSTEM_GLOB
+#  include <glob.h>
+# else
+#  include "openbsd-compat/glob.h"
+# endif
+#endif
 #include <sys/wait.h>
 #include <sys/uio.h>
 
@@ -277,6 +286,18 @@ do_cmd(char *host, char *remuser, int port, char *cmd, int *fdin, int *fdout)
 		close(pout[0]);
 		dup2(pin[0], 0);
 		dup2(pout[1], 1);
+		/*
+		 * If we're not expecting output to stderr, redirect it to void.
+		 * This helps avoiding output manipulation attacks by malicious
+		 * servers.
+		 */
+		if (!verbose_mode) {
+			int fd = open("/dev/null", O_WRONLY);
+			if (fd != -1) {
+				dup2(fd, 2);
+				close(fd);
+			}
+		}
 		close(pin[0]);
 		close(pout[1]);
 
@@ -380,9 +401,20 @@ int pflag, iamremote, iamrecursive, targetshouldbedirectory;
 #define	CMDNEEDS	64
 char cmd[CMDNEEDS];		/* must hold "rcp -r -p -d\0" */
 
+#ifdef USE_SCP_NAME_VALIDATOR
+typedef struct {
+	const char *pattern;
+	int depth;
+} SINKDATA;
+#endif
+
 int response(void);
 void rsource(char *, struct stat *);
+#ifdef USE_SCP_NAME_VALIDATOR
+void sink(int, char *[], SINKDATA *);
+#else
 void sink(int, char *[]);
+#endif
 void source(int, char *[]);
 void tolocal(int, char *[]);
 void toremote(int, char *[]);
@@ -536,7 +568,11 @@ main(int argc, char **argv)
 	}
 	if (tflag) {
 		/* Receive data. */
+#ifdef USE_SCP_NAME_VALIDATOR
+		sink(argc, argv, NULL);
+#else
 		sink(argc, argv);
+#endif
 		exit(errs != 0);
 	}
 	if (argc < 2)
@@ -750,6 +786,9 @@ tolocal(int argc, char **argv)
 	char *bp, *host = NULL, *src = NULL, *suser = NULL;
 	arglist alist;
 	int i, r, sport = -1;
+#ifdef USE_SCP_NAME_VALIDATOR
+	SINKDATA sinkdata;
+#endif
 
 	memset(&alist, '\0', sizeof(alist));
 	alist.list = NULL;
@@ -793,7 +832,13 @@ tolocal(int argc, char **argv)
 			continue;
 		}
 		free(bp);
+#ifdef USE_SCP_NAME_VALIDATOR
+		sinkdata.pattern = basename(xstrdup(src));
+		sinkdata.depth = 0;
+		sink(1, argv + argc - 1, &sinkdata);
+#else
 		sink(1, argv + argc - 1);
+#endif
 		(void) close(remin);
 		remin = remout = -1;
 	}
@@ -968,8 +1013,71 @@ rsource(char *name, struct stat *statp)
 	 (sizeof(type) == 8 && (val) > INT64_MAX) || \
 	 (sizeof(type) != 4 && sizeof(type) != 8))
 
+#ifdef USE_SCP_NAME_VALIDATOR
+#ifdef GLOB_ALTDIRFUNC
+struct fakedir {
+	struct dirent de;
+#ifdef BROKEN_ONE_BYTE_DIRENT_D_NAME
+	char denamebuf[256];
+#endif
+	struct dirent tmpde;
+#ifdef BROKEN_ONE_BYTE_DIRENT_D_NAME
+	char tmpdenamebuf[2]; /* only needs to hold "." or ".." */
+#endif
+	int dirindex;
+};
+static struct fakedir fakedir;
+static void
+g_closedir(void *ptr)
+{
+}
+static struct dirent *
+g_readdir(void *ptr)
+{
+	struct fakedir *fd = ptr;
+	switch (fd->dirindex) {
+		case 1:
+		case 2:
+			strcpy(fd->tmpde.d_name, fd->dirindex == 1 ? "." : "..");
+			fd->tmpde.d_type = DT_DIR;
+			fd->tmpde.d_ino = fd->dirindex++;
+			return &fd->tmpde;
+		case 3:
+			fd->de.d_ino = fd->dirindex++;
+			return &fd->de;
+	}
+	return NULL;
+}
+static void *
+g_opendir(const char *name)
+{
+	if (strcmp(name, ".") != 0) {
+		errno = ENOENT;
+		return NULL;
+	}
+	fakedir.dirindex = 1;
+	return &fakedir;
+}
+static int
+g_stat(const char *name, struct stat *st)
+{
+	if (strcmp(name, fakedir.de.d_name) != 0) {
+		errno = ENOENT;
+		return -1;
+	}
+	memset(st, 0, sizeof(*st));
+	st->st_mode = fakedir.de.d_type == DT_DIR ? S_IFDIR : S_IFREG;
+	return 0;
+}
+#endif
+#endif
+
 void
+#ifdef USE_SCP_NAME_VALIDATOR
+sink(int argc, char **argv, SINKDATA *sinkdata)
+#else
 sink(int argc, char **argv)
+#endif
 {
 	static BUF buffer;
 	struct stat stb;
@@ -1113,6 +1221,65 @@ sink(int argc, char **argv)
 			run_err("error: unexpected filename: %s", cp);
 			exit(1);
 		}
+
+#ifdef USE_SCP_NAME_VALIDATOR
+		if (sinkdata) {
+			/*
+			 * Validate the item name returned by the server for
+			 * attempts to modify the current directory attributes.
+			 *
+			 * Only allow it on root level and only if it was
+			 * explicitly requested by using "host:" or "dirname/."
+			 */
+			if (strcmp(cp, ".") == 0 &&
+			    (sinkdata->depth != 0 || strcmp(sinkdata->pattern, ".") != 0)) {
+				run_err("error: unexpected filename: %s", cp);
+				exit(1);
+			}
+		}
+#ifdef GLOB_ALTDIRFUNC
+		/* Use glob(3) function to validate the item name against
+		 * the last path element (stored in sinkdata->pattern).
+		 *
+		 * We verify that the items returned at the target
+		 * directory level (depth 0) match this pattern.
+		 *
+		 * While a limited check, it will prevent some of the
+		 * potential attacks by a malicious server.
+		 */
+		if (sinkdata && sinkdata->depth == 0) {
+			glob_t gl;
+			int rc;
+#ifdef BROKEN_ONE_BYTE_DIRENT_D_NAME
+			if (strlen(cp) >= 256) {
+#else
+			if (strlen(cp) >= sizeof(fakedir.de.d_name)) {
+#endif
+				run_err("error: excessively long filename: %s", cp);
+				exit(1);
+			}
+			fakedir.de.d_type = buf[0] == 'D' ? DT_DIR : DT_REG;
+			strcpy(fakedir.de.d_name, cp);
+
+			memset(&gl, 0, sizeof(gl));
+			gl.gl_closedir = g_closedir;
+			gl.gl_readdir = g_readdir;
+			gl.gl_opendir = g_opendir;
+			gl.gl_lstat = g_stat;
+			gl.gl_stat = g_stat;
+
+			rc = glob(sinkdata->pattern, GLOB_ALTDIRFUNC|GLOB_NOSORT, NULL, &gl);
+			globfree(&gl);
+			if (rc != 0) {
+				if (rc == GLOB_NOMATCH)
+					run_err("error: unexpected filename: %s", cp);
+				else
+					run_err("error: glob error %d", rc);
+				exit(1);
+			}
+		}
+#endif
+#endif
 		if (targisdir) {
 			static char *namebuf;
 			static size_t cursize;
@@ -1150,7 +1317,15 @@ sink(int argc, char **argv)
 					goto bad;
 			}
 			vect[0] = xstrdup(np);
+#ifdef USE_SCP_NAME_VALIDATOR
+			if (sinkdata)
+				sinkdata->depth++;
+			sink(1, vect, sinkdata);
+			if (sinkdata)
+				sinkdata->depth--;
+#else
 			sink(1, vect);
+#endif
 			if (setimes) {
 				setimes = 0;
 				if (utimes(vect[0], tv) < 0)

Places

File openssh-7.9p1-scp-name-validator.patch of Package openssh (Revision 92aab3767077f1ec9aa27e013729a787)

Places