From bd4850df648bee9d8e0595b7e1147266e6f55a3e Mon Sep 17 00:00:00 2001 From: Rich Salz Date: Mon, 11 Jan 2016 20:40:38 -0500 Subject: [PATCH] RT4227: Range-check in apps. Implement range-checking in all counts in apps. Turns out only a couple of cases were missing. And make the range-checking code more strict. Replace almost all opt_ulong() calls with opt_long() Reviewed-by: Viktor Dukhovni --- apps/dsaparam.c | 2 +- apps/enc.c | 6 ++- apps/opt.c | 126 ++++++++++++++++++++++-------------------------- apps/pkcs8.c | 8 +-- apps/rand.c | 4 +- 5 files changed, 67 insertions(+), 79 deletions(-) diff --git a/apps/dsaparam.c b/apps/dsaparam.c index 1689350331..c8c383faeb 100644 --- a/apps/dsaparam.c +++ b/apps/dsaparam.c @@ -180,7 +180,7 @@ int dsaparam_main(int argc, char **argv) argv = opt_rest(); if (argc == 1) { - if (!opt_int(argv[0], &num)) + if (!opt_int(argv[0], &num) || num < 0) goto end; /* generate a key */ numbits = num; diff --git a/apps/enc.c b/apps/enc.c index 58d2550d21..17cc8e8742 100644 --- a/apps/enc.c +++ b/apps/enc.c @@ -58,6 +58,7 @@ #include #include #include +#include #include "apps.h" #include #include @@ -142,7 +143,7 @@ int enc_main(int argc, char **argv) int ret = 1, inl, nopad = 0; unsigned char key[EVP_MAX_KEY_LENGTH], iv[EVP_MAX_IV_LENGTH]; unsigned char *buff = NULL, salt[PKCS5_SALT_LEN]; - unsigned long n; + long n; #ifdef ZLIB int do_zlib = 0; BIO *bzl = NULL; @@ -236,7 +237,8 @@ int enc_main(int argc, char **argv) k = i >= 1 && p[i] == 'k'; if (k) p[i] = '\0'; - if (!opt_ulong(opt_arg(), &n)) + if (!opt_long(opt_arg(), &n) + || n < 0 || (k && n >= LONG_MAX / 1024)) goto opthelp; if (k) n *= 1024; diff --git a/apps/opt.c b/apps/opt.c index 853c981ce6..1bd3965b43 100644 --- a/apps/opt.c +++ b/apps/opt.c @@ -57,6 +57,7 @@ #include #include #include +#include #include #define MAX_OPT_HELP_WIDTH 30 @@ -350,30 +351,16 @@ int opt_pair(const char *name, const OPT_PAIR* pairs, int *result) return 0; } -/* See if cp looks like a hex number, in case user left off the 0x */ -static int scanforhex(const char *cp) -{ - if (*cp == '0' && (cp[1] == 'x' || cp[1] == 'X')) - return 16; - for (; *cp; cp++) - /* Look for a hex digit that isn't a regular digit. */ - if (isxdigit(*cp) && !isdigit(*cp)) - return 16; - return 0; -} - /* Parse an int, put it into *result; return 0 on failure, else 1. */ int opt_int(const char *value, int *result) { - const char *fmt = "%d"; - int base = scanforhex(value); - - if (base == 16) - fmt = "%x"; - else if (*value == '0') - fmt = "%o"; - if (sscanf(value, fmt, result) != 1) { - BIO_printf(bio_err, "%s: Can't parse \"%s\" as a number\n", + long l; + + if (!opt_long(value, &l)) + return 0; + *result = (int)l; + if (*result != l) { + BIO_printf(bio_err, "%s: Value \"%s\" outside integer range\n", prog, value); return 0; } @@ -383,15 +370,22 @@ int opt_int(const char *value, int *result) /* Parse a long, put it into *result; return 0 on failure, else 1. */ int opt_long(const char *value, long *result) { - char *endptr; - int base = scanforhex(value); - - *result = strtol(value, &endptr, base); - if (*endptr) { - BIO_printf(bio_err, - "%s: Bad char %c in number %s\n", prog, *endptr, value); + int oerrno = errno; + long l; + char *endp; + + l = strtol(value, &endp, 0); + if (*endp + || endp == value + || ((l == LONG_MAX || l == LONG_MIN) && errno == ERANGE) + || (l == 0 && errno != 0)) { + BIO_printf(bio_err, "%s: Can't parse \"%s\" as a number\n", + prog, value); + errno = oerrno; return 0; } + *result = l; + errno = oerrno; return 1; } @@ -400,15 +394,22 @@ int opt_long(const char *value, long *result) */ int opt_ulong(const char *value, unsigned long *result) { + int oerrno = errno; char *endptr; - int base = scanforhex(value); - - *result = strtoul(value, &endptr, base); - if (*endptr) { - BIO_printf(bio_err, - "%s: Bad char %c in number %s\n", prog, *endptr, value); + unsigned long l; + + l = strtoul(value, &endptr, 0); + if (*endptr + || endptr == value + || ((l == ULONG_MAX) && errno == ERANGE) + || (l == 0 && errno != 0)) { + BIO_printf(bio_err, "%s: Can't parse \"%s\" as an unsigned number\n", + prog, value); + errno = oerrno; return 0; } + *result = l; + errno = oerrno; return 1; } @@ -421,7 +422,7 @@ enum range { OPT_V_ENUM }; int opt_verify(int opt, X509_VERIFY_PARAM *vpm) { - unsigned long ul; + long l; int i; ASN1_OBJECT *otmp; X509_PURPOSE *xptmp; @@ -468,9 +469,10 @@ int opt_verify(int opt, X509_VERIFY_PARAM *vpm) X509_VERIFY_PARAM_set_depth(vpm, i); break; case OPT_V_ATTIME: - opt_ulong(opt_arg(), &ul); - if (ul) - X509_VERIFY_PARAM_set_time(vpm, (time_t)ul); + /* If we have C99 we could use intmax_t for all time_t values */ + opt_long(opt_arg(), &l); + if (l) + X509_VERIFY_PARAM_set_time(vpm, (time_t)l); break; case OPT_V_VERIFY_HOSTNAME: if (!X509_VERIFY_PARAM_set1_host(vpm, opt_arg(), 0)) @@ -558,11 +560,9 @@ int opt_verify(int opt, X509_VERIFY_PARAM *vpm) int opt_next(void) { char *p; - char *endptr; const OPTIONS *o; - int dummy; - int base; - long val; + int ival; + unsigned long uval; /* Look at current arg; at end of the list? */ arg = NULL; @@ -613,10 +613,6 @@ int opt_next(void) } /* Syntax-check value. */ - /* - * Do some basic syntax-checking on the value. These tests aren't - * perfect (ignore range overflow) but they catch common failures. - */ switch (o->valtype) { default: case 's': @@ -645,35 +641,27 @@ int opt_next(void) return -1; case 'p': case 'n': - base = scanforhex(arg); - val = strtol(arg, &endptr, base); - if (*endptr == '\0') { - if (o->valtype == 'p' && val <= 0) { - BIO_printf(bio_err, - "%s: Non-positive number \"%s\" for -%s\n", - prog, arg, o->name); - return -1; - } - break; + if (!opt_int(arg, &ival) + || (o->valtype == 'p' && ival <= 0)) { + BIO_printf(bio_err, + "%s: Non-positive number \"%s\" for -%s\n", + prog, arg, o->name); + return -1; } - BIO_printf(bio_err, - "%s: Invalid number \"%s\" for -%s\n", - prog, arg, o->name); - return -1; + break; case 'u': - base = scanforhex(arg); - strtoul(arg, &endptr, base); - if (*endptr == '\0') - break; - BIO_printf(bio_err, - "%s: Invalid number \"%s\" for -%s\n", - prog, arg, o->name); - return -1; + if (!opt_ulong(arg, &uval)) { + BIO_printf(bio_err, + "%s: Invalid number \"%s\" for -%s\n", + prog, arg, o->name); + return -1; + } + break; case 'f': case 'F': if (opt_format(arg, o->valtype == 'F' ? OPT_FMT_PEMDER - : OPT_FMT_ANY, &dummy)) + : OPT_FMT_ANY, &ival)) break; BIO_printf(bio_err, "%s: Invalid format \"%s\" for -%s\n", diff --git a/apps/pkcs8.c b/apps/pkcs8.c index 3d7282eabb..5db78fc216 100644 --- a/apps/pkcs8.c +++ b/apps/pkcs8.c @@ -121,7 +121,7 @@ int pkcs8_main(int argc, char **argv) int informat = FORMAT_PEM, outformat = FORMAT_PEM, topk8 = 0, pbe_nid = -1; int private = 0; #ifndef OPENSSL_NO_SCRYPT - unsigned long scrypt_N = 0, scrypt_r = 0, scrypt_p = 0; + long scrypt_N = 0, scrypt_r = 0, scrypt_p = 0; #endif prog = opt_init(argc, argv, pkcs8_options); @@ -210,15 +210,15 @@ int pkcs8_main(int argc, char **argv) cipher = EVP_aes_256_cbc(); break; case OPT_SCRYPT_N: - if (!opt_ulong(opt_arg(), &scrypt_N)) + if (!opt_long(opt_arg(), &scrypt_N) || scrypt_N <= 0) goto opthelp; break; case OPT_SCRYPT_R: - if (!opt_ulong(opt_arg(), &scrypt_r)) + if (!opt_long(opt_arg(), &scrypt_r) || scrypt_r <= 0) goto opthelp; break; case OPT_SCRYPT_P: - if (!opt_ulong(opt_arg(), &scrypt_p)) + if (!opt_long(opt_arg(), &scrypt_p) || scrypt_p <= 0) goto opthelp; break; #endif diff --git a/apps/rand.c b/apps/rand.c index 150eef4fb1..bd6fdff123 100644 --- a/apps/rand.c +++ b/apps/rand.c @@ -121,9 +121,7 @@ int rand_main(int argc, char **argv) argc = opt_num_rest(); argv = opt_rest(); - if (argc != 1) - goto opthelp; - if (sscanf(argv[0], "%d", &num) != 1 || num < 0) + if (argc != 1 || !opt_int(argv[0], &num) || num < 0) goto opthelp; app_RAND_load_file(NULL, (inrand != NULL)); -- 2.25.1