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
This commit is contained in:
djm@openbsd.org 2019-01-26 22:41:28 +00:00 committed by Damien Miller
parent c2c18a3968
commit 391ffc4b9d
2 changed files with 43 additions and 12 deletions

16
scp.1
View File

@ -8,9 +8,9 @@
.\"
.\" Created: Sun May 7 00:14:37 1995 ylo
.\"
.\" $OpenBSD: scp.1,v 1.84 2019/01/22 06:58:31 jmc Exp $
.\" $OpenBSD: scp.1,v 1.85 2019/01/26 22:41:28 djm Exp $
.\"
.Dd $Mdocdate: January 22 2019 $
.Dd $Mdocdate: January 26 2019 $
.Dt SCP 1
.Os
.Sh NAME
@ -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
@ -222,6 +222,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

39
scp.c
View File

@ -1,4 +1,4 @@
/* $OpenBSD: scp.c,v 1.201 2019/01/24 16:52:17 dtucker Exp $ */
/* $OpenBSD: scp.c,v 1.202 2019/01/26 22:41:28 djm Exp $ */
/*
* scp - secure remote copy. This is basically patched BSD rcp which
* uses ssh to do the data transfer (instead of using rcmd).
@ -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,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:J:")) != -1)
fflag = Tflag = tflag = 0;
while ((ch = getopt(argc, argv,
"dfl:prtTvBCc:i:P:q12346S:o:F:J:")) != -1) {
switch (ch) {
/* User-visible flags. */
case '1':
@ -504,9 +506,13 @@ main(int argc, char **argv)
setmode(0, O_BINARY);
#endif
break;
case 'T':
Tflag = 1;
break;
default:
usage();
}
}
argc -= optind;
argv += optind;
@ -537,7 +543,7 @@ main(int argc, char **argv)
}
if (tflag) {
/* Receive data. */
sink(argc, argv);
sink(argc, argv, NULL);
exit(errs != 0);
}
if (argc < 2)
@ -795,7 +801,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;
}
@ -971,7 +977,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;
@ -987,6 +993,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]
@ -1011,6 +1018,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)
@ -1115,6 +1133,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;
@ -1152,7 +1173,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)