On Sat, 19 Jan 2019, Johnsen XXXXX wrote: > Hello, > > I would like to request an update of the progress regarding fixes > for the recently disclosed SCP vulnerability (CVE-2018-20685, > CVE-2019-6111, CVE-2019-6109, CVE-2019-6110) IMO these are all fairly intrinsic to the way that rcp/scp works. It's a crappy zombie protocol that shambled out of the 1980s. Don't use scp with untrusted servers. We've fixed one of these in git HEAD in 6010c030 and a fix for the wildcard thing is below. We're a bit wary though because, despite it being terrible, scp use is common in scripts and we don't want to break those. Testing wanted and hopefully it will make the openssh-8.0 release. We don't consider the report relating to stderr to be a vulnerability - lots of stuff depends on stderr being present (e.g. login warning banners that some people inexplicably love) and it's impractical for scp to selectively process them. The machine you just logged into can print junk to your screen, so what? We consider the last bug, relating to filename printing to be a usability problem. The reporters' patch is incorrect though, we had developed a similar one at https://bugzilla.mindrot.org/b/2434 and rejected it because it attempts unsafe operations in signal context. We'll hopefully have a fix for that in 8.0 too. -d 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..f60112bb 100644 --- a/scp.c +++ b/scp.c @@ -94,6 +94,7 @@ #include <dirent.h> #include <errno.h> #include <fcntl.h> +#include <fnmatch.h> #include <limits.h> #include <locale.h> #include <pwd.h> @@ -375,14 +376,14 @@ void verifydir(char *); struct passwd *pwd; uid_t userid; int errs, remin, remout; -int pflag, iamremote, iamrecursive, targetshouldbedirectory; +int Tflag, pflag, iamremote, iamrecursive, targetshouldbedirectory; #define CMDNEEDS 64 char cmd[CMDNEEDS]; /* must hold "rcp -r -p -d\0" */ int response(void); void rsource(char *, struct stat *); -void sink(int, char *[]); +void sink(int, char *[], const char *); void source(int, char *[]); void tolocal(int, char *[]); void toremote(int, char *[]); @@ -423,8 +424,8 @@ main(int argc, char **argv) addargs(&args, "-oRemoteCommand=none"); addargs(&args, "-oRequestTTY=no"); - fflag = tflag = 0; - while ((ch = getopt(argc, argv, "dfl:prtvBCc:i:P:q12346S:o:F:")) != -1) + fflag = Tflag = tflag = 0; + while ((ch = getopt(argc, argv, "dfl:prtTvBCc:i:P:q12346S:o:F:")) != -1) switch (ch) { /* User-visible flags. */ case '1': @@ -503,6 +504,9 @@ main(int argc, char **argv) setmode(0, O_BINARY); #endif break; + case 'T': + Tflag = 1; + break; default: usage(); } @@ -536,7 +540,7 @@ main(int argc, char **argv) } if (tflag) { /* Receive data. */ - sink(argc, argv); + sink(argc, argv, NULL); exit(errs != 0); } if (argc < 2) @@ -793,7 +797,7 @@ tolocal(int argc, char **argv) continue; } free(bp); - sink(1, argv + argc - 1); + sink(1, argv + argc - 1, src); (void) close(remin); remin = remout = -1; } @@ -969,7 +973,7 @@ rsource(char *name, struct stat *statp) (sizeof(type) != 4 && sizeof(type) != 8)) void -sink(int argc, char **argv) +sink(int argc, char **argv, const char *src) { static BUF buffer; struct stat stb; @@ -985,6 +989,7 @@ sink(int argc, char **argv) unsigned long long ull; int setimes, targisdir, wrerrno = 0; char ch, *cp, *np, *targ, *why, *vect[1], buf[2048], visbuf[2048]; + char *src_copy = NULL, *restrict_pattern = NULL; struct timeval tv[2]; #define atime tv[0] @@ -1009,6 +1014,17 @@ sink(int argc, char **argv) (void) atomicio(vwrite, remout, "", 1); if (stat(targ, &stb) == 0 && S_ISDIR(stb.st_mode)) targisdir = 1; + if (src != NULL && !iamrecursive && !Tflag) { + /* + * Prepare to try to restrict incoming filenames to match + * the requested destination file glob. + */ + if ((src_copy = strdup(src)) == NULL) + fatal("strdup failed"); + if ((restrict_pattern = strrchr(src_copy, '/')) != NULL) { + *restrict_pattern++ = '\0'; + } + } for (first = 1;; first = 0) { cp = buf; if (atomicio(read, remin, cp, 1) != 1) @@ -1113,6 +1129,9 @@ sink(int argc, char **argv) run_err("error: unexpected filename: %s", cp); exit(1); } + if (restrict_pattern != NULL && + fnmatch(restrict_pattern, cp, 0) != 0) + SCREWUP("filename does not match request"); if (targisdir) { static char *namebuf; static size_t cursize; @@ -1150,7 +1169,7 @@ sink(int argc, char **argv) goto bad; } vect[0] = xstrdup(np); - sink(1, vect); + sink(1, vect, src); if (setimes) { setimes = 0; if (utimes(vect[0], tv) < 0) _______________________________________________ openssh-unix-dev mailing list openssh-unix-dev@xxxxxxxxxxx https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
