Updated diff with changes from tobias@: This patch now removes (u_int) casts from input/output buffer allocation in dd.c, which was incorrect beforehand anyway, but with the other changes here was manifesting as a segfault for combinations of cbs and ibs/obs that overflowed u_int.
-- Scott Cheloha Index: bin/dd/args.c =================================================================== RCS file: /cvs/src/bin/dd/args.c,v retrieving revision 1.29 diff -u -p -r1.29 args.c --- bin/dd/args.c 3 Jan 2018 19:12:20 -0000 1.29 +++ bin/dd/args.c 31 Mar 2018 17:47:04 -0000 @@ -39,8 +39,10 @@ #include <err.h> #include <errno.h> +#include <ctype.h> #include <limits.h> #include <stdio.h> +#include <stdint.h> #include <stdlib.h> #include <string.h> @@ -62,6 +64,9 @@ static void f_skip(char *); static void f_status(char *); static size_t get_bsz(char *); static off_t get_off(char *); +static unsigned long long + get_expr(const char *, unsigned long long, unsigned long long); +static long long get_multiplier(const char *); static const struct arg { const char *name; @@ -94,6 +99,7 @@ jcl(char **argv) char *arg; in.dbsz = out.dbsz = 512; + files_cnt = 1; while ((oper = *++argv) != NULL) { if ((oper = strdup(oper)) == NULL) @@ -138,8 +144,6 @@ jcl(char **argv) if (ddflags & (C_BLOCK|C_UNBLOCK)) { if (!(ddflags & C_CBS)) errx(1, "record operations require cbs"); - if (cbsz == 0) - errx(1, "cbs cannot be zero"); cfunc = ddflags & C_BLOCK ? block : unblock; } else if (ddflags & C_CBS) { if (ddflags & (C_ASCII|C_EBCDIC)) { @@ -152,23 +156,14 @@ jcl(char **argv) } } else errx(1, "cbs meaningless if not doing record operations"); - if (cbsz == 0) - errx(1, "cbs cannot be zero"); } else cfunc = def; - if (in.dbsz == 0 || out.dbsz == 0) - errx(1, "buffer sizes cannot be zero"); - - /* - * Read and write take size_t's as arguments. Lseek, however, - * takes an off_t. - */ - if (cbsz > SSIZE_MAX || in.dbsz > SSIZE_MAX || out.dbsz > SSIZE_MAX) - errx(1, "buffer sizes cannot be greater than %zd", - (ssize_t)SSIZE_MAX); - if (in.offset > LLONG_MAX / in.dbsz || out.offset > LLONG_MAX / out.dbsz) - errx(1, "seek offsets cannot be larger than %lld", LLONG_MAX); + /* Ensure the seek won't overflow. */ + if (in.offset != 0 && in.dbsz > LLONG_MAX / in.offset) + errx(1, "total skip is too large"); + if (out.offset != 0 && out.dbsz > LLONG_MAX / out.offset) + errx(1, "total seek is too large"); } static int @@ -196,15 +191,14 @@ static void f_count(char *arg) { - if ((cpy_cnt = get_bsz(arg)) == 0) - cpy_cnt = (size_t)-1; + cpy_cnt = get_expr(arg, 0, ULLONG_MAX); } static void f_files(char *arg) { - files_cnt = get_bsz(arg); + files_cnt = get_expr(arg, 0, ULLONG_MAX); } static void @@ -307,165 +301,108 @@ f_conv(char *arg) } } -/* - * Convert an expression of the following forms to a size_t - * 1) A positive decimal number, optionally followed by - * b - multiply by 512. - * k, m or g - multiply by 1024 each. - * w - multiply by sizeof int - * 2) Two or more of the above, separated by x - * (or * for backwards compatibility), specifying - * the product of the indicated values. - */ static size_t -get_bsz(char *val) +get_bsz(char *arg) { - size_t num, t; - char *expr; + /* read(2) and write(2) can't handle more than SSIZE_MAX bytes. */ + return get_expr(arg, 1, SSIZE_MAX); +} - if (strchr(val, '-')) - errx(1, "%s: illegal numeric value", oper); +static off_t +get_off(char *arg) +{ + return get_expr(arg, 0, LLONG_MAX); +} - errno = 0; - num = strtoul(val, &expr, 0); - if (num == ULONG_MAX && errno == ERANGE) /* Overflow. */ - err(1, "%s", oper); - if (expr == val) /* No digits. */ - errx(1, "%s: illegal numeric value", oper); +/* + * An expression is composed of one or more terms separated by exes ('x'). + * Terms separated by asterisks ('*') are supported for backwards + * compatibility. The value of an expression is the product of its terms. + * + * A term is composed of a non-negative decimal integer and an optional + * suffix of a single character. Suffixes are valued as follows: + * + * b - 512 ("block") + * G|g - 1073741824 ("gigabyte") + * K|k - 1024 ("kilobyte") + * M|m - 1048576 ("megabyte") + * w - sizeof(int) ("word") + * + * The value of a term without a suffix is that of its integer. The value + * of a term with a suffix is the product of its integer and its suffix. + */ +static unsigned long long +get_expr(const char *arg, unsigned long long min, unsigned long long max) +{ + unsigned long long number, value; + long long multiplier; + char *expr, *term, *suffix; - switch(*expr) { - case 'b': - t = num; - num *= 512; - if (t > num) - goto erange; - ++expr; - break; - case 'g': - case 'G': - t = num; - num *= 1024; - if (t > num) - goto erange; - /* fallthrough */ - case 'm': - case 'M': - t = num; - num *= 1024; - if (t > num) - goto erange; - /* fallthrough */ - case 'k': - case 'K': - t = num; - num *= 1024; - if (t > num) - goto erange; - ++expr; - break; - case 'w': - t = num; - num *= sizeof(int); - if (t > num) - goto erange; - ++expr; - break; - } + value = 1; + + expr = strdup(arg); + if (expr == NULL) + err(1, NULL); - switch(*expr) { - case '\0': - break; - case '*': /* Backward compatible. */ - case 'x': - t = num; - num *= get_bsz(expr + 1); - if (t > num) - goto erange; - break; - default: - errx(1, "%s: illegal numeric value", oper); + while ((term = strsep(&expr, "*x")) != NULL) { + /* + * strtoull(3) ignores leading whitespace and quietly + * underflows terms prefixed with a '-', so we have to check. + */ + if (!isdigit((unsigned char)term[0])) + errx(1, "%s is invalid: %s", oper, arg); + errno = 0; + number = strtoull(term, &suffix, 10); + if (term[0] == '\0' || suffix == term) + errx(1, "%s is invalid: %s", oper, arg); + if (number < min) + errx(1, "%s is too small: %s", oper, arg); + if (number > max || (number == ULLONG_MAX && errno == ERANGE)) + errx(1, "%s is too large: %s", oper, arg); + multiplier = get_multiplier(suffix); + if (multiplier == -1) + errx(1, "%s is invalid: %s", oper, arg); + if (number > max / multiplier || + (value > 0 && number * multiplier > max / value)) + errx(1, "%s is too large: %s", oper, arg); + value *= number * multiplier; } - return (num); -erange: - errc(1, ERANGE, "%s", oper); + free(expr); + return (value); } -/* - * Convert an expression of the following forms to an off_t - * 1) A positive decimal number, optionally followed by - * b - multiply by 512. - * k, m or g - multiply by 1024 each. - * w - multiply by sizeof int - * 2) Two or more of the above, separated by x - * (or * for backwards compatibility), specifying - * the product of the indicated values. - */ -static off_t -get_off(char *val) +static long long +get_multiplier(const char *str) { - off_t num, t; - char *expr; - - errno = 0; - num = strtoll(val, &expr, 0); - if (num == LLONG_MAX && errno == ERANGE) /* Overflow. */ - err(1, "%s", oper); - if (expr == val) /* No digits. */ - errx(1, "%s: illegal numeric value", oper); + long long value; + int unknown = 0; - switch(*expr) { + switch(str[0]) { + case '\0': + value = 1; + break; case 'b': - t = num; - num *= 512; - if (t > num) - goto erange; - ++expr; + value = 512; break; - case 'g': case 'G': - t = num; - num *= 1024; - if (t > num) - goto erange; - /* fallthrough */ - case 'm': - case 'M': - t = num; - num *= 1024; - if (t > num) - goto erange; - /* fallthrough */ - case 'k': + case 'g': + value = 1073741824LL; + break; case 'K': - t = num; - num *= 1024; - if (t > num) - goto erange; - ++expr; + case 'k': + value = 1024; + break; + case 'M': + case 'm': + value = 1048576; break; case 'w': - t = num; - num *= sizeof(int); - if (t > num) - goto erange; - ++expr; + value = sizeof(int); break; + default: + unknown = 1; } - - switch(*expr) { - case '\0': - break; - case '*': /* Backward compatible. */ - case 'x': - t = num; - num *= get_off(expr + 1); - if (t > num) - goto erange; - break; - default: - errx(1, "%s: illegal numeric value", oper); - } - return (num); -erange: - errc(1, ERANGE, "%s", oper); + if (unknown || (str[0] != '\0' && str[1] != '\0')) + value = -1; + return (value); } Index: bin/dd/dd.c =================================================================== RCS file: /cvs/src/bin/dd/dd.c,v retrieving revision 1.24 diff -u -p -r1.24 dd.c --- bin/dd/dd.c 13 Aug 2017 02:06:42 -0000 1.24 +++ bin/dd/dd.c 31 Mar 2018 17:47:04 -0000 @@ -62,10 +62,10 @@ static void setup(void); IO in, out; /* input/output state */ STAT st; /* statistics */ void (*cfunc)(void); /* conversion function */ -size_t cpy_cnt; /* # of blocks to copy */ +unsigned long long cpy_cnt; /* # of blocks to copy */ u_int ddflags; /* conversion options */ size_t cbsz; /* conversion block size */ -size_t files_cnt = 1; /* # of files to copy */ +unsigned long long files_cnt; /* # of files to copy */ const u_char *ctab; /* conversion table */ int @@ -79,7 +79,7 @@ main(int argc, char *argv[]) atexit(summary); - if (cpy_cnt != (size_t)-1) { + if (!(ddflags & C_COUNT) || cpy_cnt > 0) { while (files_cnt--) dd_in(); } @@ -102,7 +102,7 @@ setup(void) getfdtype(&in); - if (files_cnt > 1 && !(in.flags & ISTAPE)) + if ((ddflags & C_FILES) && !(in.flags & ISTAPE)) errx(1, "files is not supported for non-tape devices"); if (out.name == NULL) { @@ -136,10 +136,14 @@ setup(void) if ((in.db = malloc(out.dbsz + in.dbsz - 1)) == NULL) err(1, "input buffer"); out.db = in.db; - } else if ((in.db = - malloc((u_int)(MAXIMUM(in.dbsz, cbsz) + cbsz))) == NULL || - (out.db = malloc((u_int)(out.dbsz + cbsz))) == NULL) - err(1, "output buffer"); + } else { + in.db = malloc(MAXIMUM(in.dbsz, cbsz) + cbsz); + if (in.db == NULL) + err(1, "input buffer"); + out.db = malloc(out.dbsz + cbsz); + if (out.db == NULL) + err(1, "output buffer"); + } in.dbp = in.db; out.dbp = out.db; @@ -232,7 +236,7 @@ dd_in(void) ssize_t n; for (;;) { - if (cpy_cnt && (st.in_full + st.in_part) >= cpy_cnt) + if ((ddflags & C_COUNT) && st.in_full + st.in_part >= cpy_cnt) return; /* Index: bin/dd/extern.h =================================================================== RCS file: /cvs/src/bin/dd/extern.h,v retrieving revision 1.9 diff -u -p -r1.9 extern.h --- bin/dd/extern.h 27 Mar 2014 15:32:13 -0000 1.9 +++ bin/dd/extern.h 31 Mar 2018 17:47:04 -0000 @@ -53,10 +53,10 @@ void unblock_close(void); extern IO in, out; extern STAT st; extern void (*cfunc)(void); -extern size_t cpy_cnt; +extern unsigned long long cpy_cnt; extern size_t cbsz; extern u_int ddflags; -extern size_t files_cnt; +extern unsigned long long files_cnt; extern const u_char *ctab; extern const u_char a2e_POSIX[]; extern const u_char e2a_POSIX[];