Is sshd supposed to interpret "{a,b}" brace expansions?
Damien Miller
djm at mindrot.org
Sun Feb 3 10:30:10 AEDT 2019
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);
}
More information about the openssh-unix-dev
mailing list