blob: e498da381941414dadfa82218120da27b8fc4375 [file] [log] [blame]
From 15cc3497367d2e9729353b3df75518548e845c82 Mon Sep 17 00:00:00 2001
From: "djm@openbsd.org" <djm@openbsd.org>
Date: Sat, 26 Jan 2019 22:41:28 +0000
Subject: [PATCH] upstream: check in scp client that filenames sent during
remote->local directory copies satisfy the wildcard specified by the user.
This checking provides some protection against a malicious server
sending unexpected filenames, but it comes at a risk of rejecting wanted
files due to differences between client and server wildcard expansion rules.
For this reason, this also adds a new -T flag to disable the check.
reported by Harry Sintonen
fix approach suggested by markus@;
has been in snaps for ~1wk courtesy deraadt@
OpenBSD-Commit-ID: 00f44b50d2be8e321973f3c6d014260f8f7a8eda
CVE: CVE-2019-6111
Upstream-Status: Backport
Signed-off-by: Anuj Mittal <anuj.mittal@intel.com>
---
scp.1 | 12 +++++++++++-
scp.c | 37 +++++++++++++++++++++++++++++--------
2 files changed, 40 insertions(+), 9 deletions(-)
diff --git a/scp.1 b/scp.1
index 0e5cc1b..397e770 100644
--- a/scp.1
+++ b/scp.1
@@ -18,7 +18,7 @@
.Nd secure copy (remote file copy program)
.Sh SYNOPSIS
.Nm scp
-.Op Fl 346BCpqrv
+.Op Fl 346BCpqrTv
.Op Fl c Ar cipher
.Op Fl F Ar ssh_config
.Op Fl i Ar identity_file
@@ -208,6 +208,16 @@ to use for the encrypted connection.
The program must understand
.Xr ssh 1
options.
+.It Fl T
+Disable strict filename checking.
+By default when copying files from a remote host to a local directory
+.Nm
+checks that the received filenames match those requested on the command-line
+to prevent the remote end from sending unexpected or unwanted files.
+Because of differences in how various operating systems and shells interpret
+filename wildcards, these checks may cause wanted files to be rejected.
+This option disables these checks at the expense of fully trusting that
+the server will not send unexpected filenames.
.It Fl v
Verbose mode.
Causes
diff --git a/scp.c b/scp.c
index 0587cec..b2d331e 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 *[]);
@@ -421,8 +422,9 @@ 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':
@@ -501,9 +503,13 @@ main(int argc, char **argv)
setmode(0, O_BINARY);
#endif
break;
+ case 'T':
+ Tflag = 1;
+ break;
default:
usage();
}
+ }
argc -= optind;
argv += optind;
@@ -534,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)
@@ -792,7 +798,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;
}
@@ -968,7 +974,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;
@@ -984,6 +990,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]
@@ -1008,6 +1015,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)
@@ -1112,6 +1130,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;
@@ -1149,7 +1170,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)
--
2.7.4