On Thu, 31 Jan 2019, Peter Simons wrote: > Jakub Jelen writes: > > > from what I understand, the brace expansion is not expanded in the > > remote scp nor sshd, but in the remote shell (the remote command is > > run inside of bash -c "command"). > > yes, you are right of course. Thank you for pointing that out. > > > Damien Miller writes: > > >> the proposed fix for CVE-2019-6111 [1] adds file name validation to > >> scp [...] > > > > That's _a_ proposed fix, but not the one we used. > > > > Ours is: https://anongit.mindrot.org/openssh.git/patch/?id=391ffc4b9 > > I see. Thank you very much for the pointer. You can try this patch to git HEAD. I'm interested to hear test reports. diff --git a/scp.c b/scp.c index 44ac917..235bbb7 100644 --- a/scp.c +++ b/scp.c @@ -605,6 +605,253 @@ parse_scp_uri(const char *uri, char **userp, char **hostp, int *portp, return r; } +/* Appends a string to an array; returns 0 on success, -1 on alloc failure */ +static int +append(char *cp, char ***ap, size_t *np) +{ + char **tmp; + + if ((tmp = reallocarray(*ap, *np + 1, sizeof(*tmp))) == NULL) + return -1; + tmp[(*np)] = cp; + (*np)++; + *ap = tmp; + return 0; +} + +/* + * Finds the start and end of the first brace pair in the pattern. + * returns 0 on success or -1 for invalid patterns. + */ +static int +find_brace(const char *pattern, int *startp, int *endp) +{ + int i; + int in_bracket, brace_level; + + *startp = *endp = -1; + in_bracket = brace_level = 0; + for (i = 0; i < INT_MAX && *endp < 0 && pattern[i] != '\0'; i++) { + switch (pattern[i]) { + case '\\': + /* skip next character */ + if (pattern[i + 1] != '\0') + i++; + break; + case '[': + in_bracket = 1; + break; + case ']': + in_bracket = 0; + break; + case '{': + if (in_bracket) + break; + if (pattern[i + 1] == '}') { + /* Protect a single {}, for find(1), like csh */ + i++; /* skip */ + break; + } + if (*startp == -1) + *startp = i; + brace_level++; + break; + case '}': + if (in_bracket) + break; + if (*startp < 0) { + /* Unbalanced brace */ + return -1; + } + if (--brace_level <= 0) + *endp = i; + break; + } + } + /* unbalanced brackets/braces */ + if (*endp < 0 && (*startp >= 0 || in_bracket)) + return -1; + return 0; +} + +/* + * Assembles and records a successfully-expanded pattern, returns -1 on + * alloc failure. + */ +static int +emit_expansion(const char *pattern, int brace_start, int brace_end, + int sel_start, int sel_end, char ***patternsp, size_t *npatternsp) +{ + char *cp; + int o = 0, tail_len = strlen(pattern + brace_end + 1); + + if ((cp = malloc(brace_start + (sel_end - sel_start) + + tail_len + 1)) == NULL) + return -1; + + /* Pattern before initial brace */ + if (brace_start > 0) { + memcpy(cp, pattern, brace_start); + o = brace_start; + } + /* Current braced selection */ + if (sel_end - sel_start > 0) { + memcpy(cp + o, pattern + sel_start, + sel_end - sel_start); + o += sel_end - sel_start; + } + /* Remainder of pattern after closing brace */ + if (tail_len > 0) { + memcpy(cp + o, pattern + brace_end + 1, tail_len); + o += tail_len; + } + cp[o] = '\0'; + if (append(cp, patternsp, npatternsp) != 0) { + free(cp); + return -1; + } + return 0; +} + +/* + * Expand the first encountered brace in pattern, appending the expanded + * patterns it yielded to the *patternsp array. + * + * Returns 0 on success or -1 on allocation failure. + * + * Signals whether expansion was performed via *expanded and whether + * pattern was invalid via *invalid. + */ +static int +brace_expand_one(const char *pattern, char ***patternsp, size_t *npatternsp, + int *expanded, int *invalid) +{ + int i; + int in_bracket, brace_start, brace_end, brace_level; + int sel_start, sel_end; + + *invalid = *expanded = 0; + + if (find_brace(pattern, &brace_start, &brace_end) != 0) { + *invalid = 1; + return 0; + } else if (brace_start == -1) + return 0; + + in_bracket = brace_level = 0; + for (i = sel_start = brace_start + 1; i < brace_end; i++) { + switch (pattern[i]) { + case '{': + if (in_bracket) + break; + brace_level++; + break; + case '}': + if (in_bracket) + break; + brace_level--; + break; + case '[': + in_bracket = 1; + break; + case ']': + in_bracket = 0; + break; + case '\\': + if (i < brace_end - 1) + i++; /* skip */ + break; + } + if (pattern[i] == ',' || i == brace_end - 1) { + if (in_bracket || brace_level > 0) + continue; + /* End of a selection, emit an expanded pattern */ + + /* Adjust end index for last selection */ + sel_end = (i == brace_end - 1) ? brace_end : i; + if (emit_expansion(pattern, brace_start, brace_end, + sel_start, sel_end, patternsp, npatternsp) != 0) + return -1; + /* move on to the next selection */ + sel_start = i + 1; + continue; + } + } + if (in_bracket || brace_level > 0) { + *invalid = 1; + return 0; + } + /* success */ + *expanded = 1; + return 0; +} + +/* Expand braces from pattern. Returns 0 on success, -1 on failure */ +static int +brace_expand(const char *pattern, char ***patternsp, size_t *npatternsp) +{ + char *cp, *cp2, **active = NULL, **done = NULL; + size_t i, nactive = 0, ndone = 0; + int ret = -1, invalid = 0, expanded = 0; + + *patternsp = NULL; + *npatternsp = 0; + + /* Start the worklist with the original pattern */ + if ((cp = strdup(pattern)) == NULL) + return -1; + if (append(cp, &active, &nactive) != 0) { + free(cp); + return -1; + } + while (nactive > 0) { + cp = active[nactive - 1]; + nactive--; + if (brace_expand_one(cp, &active, &nactive, + &expanded, &invalid) == -1) { + free(cp); + goto fail; + } + if (invalid) + fatal("%s: invalid brace pattern \"%s\"", __func__, cp); + if (expanded) { + /* + * Current entry expanded to new entries on the + * active list; discard the progenitor pattern. + */ + free(cp); + continue; + } + /* + * Pattern did not expand; append the finename component to + * the completed list + */ + if ((cp2 = strrchr(cp, '/')) != NULL) + *cp2++ = '\0'; + else + cp2 = cp; + if (append(xstrdup(cp2), &done, &ndone) != 0) { + free(cp); + goto fail; + } + free(cp); + } + /* success */ + *patternsp = done; + *npatternsp = ndone; + done = NULL; + ndone = 0; + ret = 0; + fail: + for (i = 0; i < nactive; i++) + free(active[i]); + free(active); + for (i = 0; i < ndone; i++) + free(done[i]); + free(done); + return ret; +} + void toremote(int argc, char **argv) { @@ -968,7 +1215,8 @@ sink(int argc, char **argv, const char *src) 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; + char **patterns = NULL; + size_t n, npatterns = 0; struct timeval tv[2]; #define atime tv[0] @@ -998,16 +1246,13 @@ sink(int argc, char **argv, const char *src) * 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'; - } + if (brace_expand(src, &patterns, &npatterns) != 0) + fatal("%s: could not expand pattern", __func__); } for (first = 1;; first = 0) { cp = buf; if (atomicio(read, remin, cp, 1) != 1) - return; + goto done; if (*cp++ == '\n') SCREWUP("unexpected <newline>"); do { @@ -1033,7 +1278,7 @@ sink(int argc, char **argv, const char *src) } if (buf[0] == 'E') { (void) atomicio(vwrite, remout, "", 1); - return; + goto done; } if (ch == '\n') *--cp = 0; @@ -1108,9 +1353,14 @@ sink(int argc, char **argv, const char *src) 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 (npatterns > 0) { + for (n = 0; n < npatterns; n++) { + if (fnmatch(patterns[n], cp, 0) == 0) + break; + } + if (n >= npatterns) + SCREWUP("filename does not match request"); + } if (targisdir) { static char *namebuf; static size_t cursize; @@ -1261,7 +1511,15 @@ bad: run_err("%s: %s", np, strerror(errno)); break; } } +done: + for (n = 0; n < npatterns; n++) + free(patterns[n]); + free(patterns); + return; screwup: + for (n = 0; n < npatterns; n++) + free(patterns[n]); + free(patterns); run_err("protocol error: %s", why); exit(1); } _______________________________________________ openssh-unix-dev mailing list openssh-unix-dev@xxxxxxxxxxx https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
