forked from pool/openssh
be528d6e10
- Security fix:
* [bsc#1121816, CVE-2019-6109] scp client spoofing via object name
* [bsc#1121818, CVE-2019-6110] scp client spoofing via stderr
* [bsc#1121821, CVE-2019-6111] scp client missing received object
name validation
* Added patch openssh-7.9p1-scp-name-validator.patch
OBS-URL: https://build.opensuse.org/request/show/666511
OBS-URL: https://build.opensuse.org/package/show/network/openssh?expand=0&rev=170
349 lines
8.5 KiB
Diff
349 lines
8.5 KiB
Diff
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)
|