d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

scp: validate filenames provided by server against wildcard in client

Browse Source 
OpenSSH-portable commits:

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

Minor patch conflict (getopt) resolved.

Obtained from: OpenSSH-portable 391ffc4b9d31fa1f4ad566499fef9176ff8a07dc

scp: add -T to usage();

OpenBSD-Commit-ID: a7ae14d9436c64e1bd05022329187ea3a0ce1899

Obtained from: OpenSSH-portable 2c21b75a7be6ebdcbceaebb43157c48dbb36f3d8

PR:		234965
Approved by:	des
MFC after:	3 days
Obtained from:	OpenSSH-portable 391ffc4b9d, 2c21b75a7b
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D19076
This commit is contained in:
Ed Maste 2019-02-21 22:45:54 +00:00
commit afde517053
2 changed files with 42 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
crypto/openssh/scp.1
View File

@ -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
@ -207,6 +207,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

41
crypto/openssh/scp.c
View File

@ -1,4 +1,4 @@
/* $OpenBSD: scp.c,v 1.197 2018/06/01 04:31:48 dtucker Exp $ */
/* $OpenBSD: scp.c,v 1.203 2019/01/27 07:14:11 jmc 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 *[]);
@ -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)
@ -791,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;
}
@ -967,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;
@ -983,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]
@ -1007,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)
@ -1111,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;
@ -1148,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)
@ -1316,7 +1337,7 @@ void
usage(void)
{
(void) fprintf(stderr,
"usage: scp [-346BCpqrv] [-c cipher] [-F ssh_config] [-i identity_file]\n"
"usage: scp [-346BCpqrTv] [-c cipher] [-F ssh_config] [-i identity_file]\n"
" [-l limit] [-o ssh_option] [-P port] [-S program] source ... target\n");
exit(1);
}