Brad Bishop | c342db3 | 2019-05-15 21:57:59 -0400 | [diff] [blame] | 1 | From 15cc3497367d2e9729353b3df75518548e845c82 Mon Sep 17 00:00:00 2001 |
| 2 | From: "djm@openbsd.org" <djm@openbsd.org> |
| 3 | Date: Sat, 26 Jan 2019 22:41:28 +0000 |
| 4 | Subject: [PATCH] upstream: check in scp client that filenames sent during |
| 5 | |
| 6 | remote->local directory copies satisfy the wildcard specified by the user. |
| 7 | |
| 8 | This checking provides some protection against a malicious server |
| 9 | sending unexpected filenames, but it comes at a risk of rejecting wanted |
| 10 | files due to differences between client and server wildcard expansion rules. |
| 11 | |
| 12 | For this reason, this also adds a new -T flag to disable the check. |
| 13 | |
| 14 | reported by Harry Sintonen |
| 15 | fix approach suggested by markus@; |
| 16 | has been in snaps for ~1wk courtesy deraadt@ |
| 17 | |
| 18 | OpenBSD-Commit-ID: 00f44b50d2be8e321973f3c6d014260f8f7a8eda |
| 19 | |
| 20 | CVE: CVE-2019-6111 |
| 21 | Upstream-Status: Backport |
| 22 | Signed-off-by: Anuj Mittal <anuj.mittal@intel.com> |
| 23 | --- |
| 24 | scp.1 | 12 +++++++++++- |
| 25 | scp.c | 37 +++++++++++++++++++++++++++++-------- |
| 26 | 2 files changed, 40 insertions(+), 9 deletions(-) |
| 27 | |
| 28 | diff --git a/scp.1 b/scp.1 |
| 29 | index 0e5cc1b..397e770 100644 |
| 30 | --- a/scp.1 |
| 31 | +++ b/scp.1 |
| 32 | @@ -18,7 +18,7 @@ |
| 33 | .Nd secure copy (remote file copy program) |
| 34 | .Sh SYNOPSIS |
| 35 | .Nm scp |
| 36 | -.Op Fl 346BCpqrv |
| 37 | +.Op Fl 346BCpqrTv |
| 38 | .Op Fl c Ar cipher |
| 39 | .Op Fl F Ar ssh_config |
| 40 | .Op Fl i Ar identity_file |
| 41 | @@ -208,6 +208,16 @@ to use for the encrypted connection. |
| 42 | The program must understand |
| 43 | .Xr ssh 1 |
| 44 | options. |
| 45 | +.It Fl T |
| 46 | +Disable strict filename checking. |
| 47 | +By default when copying files from a remote host to a local directory |
| 48 | +.Nm |
| 49 | +checks that the received filenames match those requested on the command-line |
| 50 | +to prevent the remote end from sending unexpected or unwanted files. |
| 51 | +Because of differences in how various operating systems and shells interpret |
| 52 | +filename wildcards, these checks may cause wanted files to be rejected. |
| 53 | +This option disables these checks at the expense of fully trusting that |
| 54 | +the server will not send unexpected filenames. |
| 55 | .It Fl v |
| 56 | Verbose mode. |
| 57 | Causes |
| 58 | diff --git a/scp.c b/scp.c |
| 59 | index 0587cec..b2d331e 100644 |
| 60 | --- a/scp.c |
| 61 | +++ b/scp.c |
| 62 | @@ -94,6 +94,7 @@ |
| 63 | #include <dirent.h> |
| 64 | #include <errno.h> |
| 65 | #include <fcntl.h> |
| 66 | +#include <fnmatch.h> |
| 67 | #include <limits.h> |
| 68 | #include <locale.h> |
| 69 | #include <pwd.h> |
| 70 | @@ -375,14 +376,14 @@ void verifydir(char *); |
| 71 | struct passwd *pwd; |
| 72 | uid_t userid; |
| 73 | int errs, remin, remout; |
| 74 | -int pflag, iamremote, iamrecursive, targetshouldbedirectory; |
| 75 | +int Tflag, pflag, iamremote, iamrecursive, targetshouldbedirectory; |
| 76 | |
| 77 | #define CMDNEEDS 64 |
| 78 | char cmd[CMDNEEDS]; /* must hold "rcp -r -p -d\0" */ |
| 79 | |
| 80 | int response(void); |
| 81 | void rsource(char *, struct stat *); |
| 82 | -void sink(int, char *[]); |
| 83 | +void sink(int, char *[], const char *); |
| 84 | void source(int, char *[]); |
| 85 | void tolocal(int, char *[]); |
| 86 | void toremote(int, char *[]); |
| 87 | @@ -421,8 +422,9 @@ main(int argc, char **argv) |
| 88 | addargs(&args, "-oRemoteCommand=none"); |
| 89 | addargs(&args, "-oRequestTTY=no"); |
| 90 | |
| 91 | - fflag = tflag = 0; |
| 92 | - while ((ch = getopt(argc, argv, "dfl:prtvBCc:i:P:q12346S:o:F:")) != -1) |
| 93 | + fflag = Tflag = tflag = 0; |
| 94 | + while ((ch = getopt(argc, argv, |
| 95 | + "dfl:prtTvBCc:i:P:q12346S:o:F:")) != -1) { |
| 96 | switch (ch) { |
| 97 | /* User-visible flags. */ |
| 98 | case '1': |
| 99 | @@ -501,9 +503,13 @@ main(int argc, char **argv) |
| 100 | setmode(0, O_BINARY); |
| 101 | #endif |
| 102 | break; |
| 103 | + case 'T': |
| 104 | + Tflag = 1; |
| 105 | + break; |
| 106 | default: |
| 107 | usage(); |
| 108 | } |
| 109 | + } |
| 110 | argc -= optind; |
| 111 | argv += optind; |
| 112 | |
| 113 | @@ -534,7 +540,7 @@ main(int argc, char **argv) |
| 114 | } |
| 115 | if (tflag) { |
| 116 | /* Receive data. */ |
| 117 | - sink(argc, argv); |
| 118 | + sink(argc, argv, NULL); |
| 119 | exit(errs != 0); |
| 120 | } |
| 121 | if (argc < 2) |
| 122 | @@ -792,7 +798,7 @@ tolocal(int argc, char **argv) |
| 123 | continue; |
| 124 | } |
| 125 | free(bp); |
| 126 | - sink(1, argv + argc - 1); |
| 127 | + sink(1, argv + argc - 1, src); |
| 128 | (void) close(remin); |
| 129 | remin = remout = -1; |
| 130 | } |
| 131 | @@ -968,7 +974,7 @@ rsource(char *name, struct stat *statp) |
| 132 | (sizeof(type) != 4 && sizeof(type) != 8)) |
| 133 | |
| 134 | void |
| 135 | -sink(int argc, char **argv) |
| 136 | +sink(int argc, char **argv, const char *src) |
| 137 | { |
| 138 | static BUF buffer; |
| 139 | struct stat stb; |
| 140 | @@ -984,6 +990,7 @@ sink(int argc, char **argv) |
| 141 | unsigned long long ull; |
| 142 | int setimes, targisdir, wrerrno = 0; |
| 143 | char ch, *cp, *np, *targ, *why, *vect[1], buf[2048], visbuf[2048]; |
| 144 | + char *src_copy = NULL, *restrict_pattern = NULL; |
| 145 | struct timeval tv[2]; |
| 146 | |
| 147 | #define atime tv[0] |
| 148 | @@ -1008,6 +1015,17 @@ sink(int argc, char **argv) |
| 149 | (void) atomicio(vwrite, remout, "", 1); |
| 150 | if (stat(targ, &stb) == 0 && S_ISDIR(stb.st_mode)) |
| 151 | targisdir = 1; |
| 152 | + if (src != NULL && !iamrecursive && !Tflag) { |
| 153 | + /* |
| 154 | + * Prepare to try to restrict incoming filenames to match |
| 155 | + * the requested destination file glob. |
| 156 | + */ |
| 157 | + if ((src_copy = strdup(src)) == NULL) |
| 158 | + fatal("strdup failed"); |
| 159 | + if ((restrict_pattern = strrchr(src_copy, '/')) != NULL) { |
| 160 | + *restrict_pattern++ = '\0'; |
| 161 | + } |
| 162 | + } |
| 163 | for (first = 1;; first = 0) { |
| 164 | cp = buf; |
| 165 | if (atomicio(read, remin, cp, 1) != 1) |
| 166 | @@ -1112,6 +1130,9 @@ sink(int argc, char **argv) |
| 167 | run_err("error: unexpected filename: %s", cp); |
| 168 | exit(1); |
| 169 | } |
| 170 | + if (restrict_pattern != NULL && |
| 171 | + fnmatch(restrict_pattern, cp, 0) != 0) |
| 172 | + SCREWUP("filename does not match request"); |
| 173 | if (targisdir) { |
| 174 | static char *namebuf; |
| 175 | static size_t cursize; |
| 176 | @@ -1149,7 +1170,7 @@ sink(int argc, char **argv) |
| 177 | goto bad; |
| 178 | } |
| 179 | vect[0] = xstrdup(np); |
| 180 | - sink(1, vect); |
| 181 | + sink(1, vect, src); |
| 182 | if (setimes) { |
| 183 | setimes = 0; |
| 184 | if (utimes(vect[0], tv) < 0) |
| 185 | -- |
| 186 | 2.7.4 |
| 187 | |