File openssh-6.6p1-scp-name-validator.patch of Package openssh.9942
Index: openssh-6.6p1/defines.h
===================================================================
--- openssh-6.6p1.orig/defines.h
+++ openssh-6.6p1/defines.h
@@ -826,4 +826,10 @@ struct winsize {
# define arc4random_stir()
#endif
+/*
+ * Define to enable additional scp file name validation against
+ * malicious servers.
+ */
+#define USE_SCP_NAME_VALIDATOR 1
+
#endif /* _DEFINES_H */
Index: openssh-6.6p1/regress/scp-ssh-wrapper.sh
===================================================================
--- openssh-6.6p1.orig/regress/scp-ssh-wrapper.sh
+++ openssh-6.6p1/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
Index: openssh-6.6p1/regress/scp.sh
===================================================================
--- openssh-6.6p1.orig/regress/scp.sh
+++ openssh-6.6p1/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"
Index: openssh-6.6p1/scp.c
===================================================================
--- openssh-6.6p1.orig/scp.c
+++ openssh-6.6p1/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
@@ -88,6 +89,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>
@@ -265,6 +274,18 @@ do_cmd(char *host, char *remuser, char *
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]);
@@ -357,9 +378,20 @@ int pflag, iamremote, iamrecursive, targ
#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(char *, int, char *[]);
@@ -494,7 +526,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)
@@ -694,7 +730,9 @@ tolocal(int argc, char **argv)
char *bp, *host, *src, *suser;
arglist alist;
int i;
-
+#ifdef USE_SCP_NAME_VALIDATOR
+ SINKDATA sinkdata;
+#endif
memset(&alist, '\0', sizeof(alist));
alist.list = NULL;
@@ -734,7 +772,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;
}
@@ -898,8 +942,71 @@ rsource(char *name, struct stat *statp)
(void) response();
}
+#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;
@@ -1032,6 +1139,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;
@@ -1069,7 +1235,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)