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 <viktor@openssl.org>

diff --git a/apps/dsaparam.c b/apps/dsaparam.c
index 1689350331a083eaa10d5cf8e4e570d3a04a2590..c8c383faeb301c9f9fdd0fc78063b447a5aa301f 100644 (file)
--- 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 58d2550d213e0879f82c7744bb183a993445a0ae..17cc8e8742ff1a9cbde7084b5c62ef6807626e0e 100644 (file)
--- a/apps/enc.c
+++ b/apps/enc.c
@@ -58,6 +58,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <limits.h>
 #include "apps.h"
 #include <openssl/bio.h>
 #include <openssl/err.h>
@@ -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 853c981ce6be25b74802692e63baafb1e63c59d6..1bd3965b438c10c05277daba3a83b95eb1449dd5 100644 (file)
--- a/apps/opt.c
+++ b/apps/opt.c
@@ -57,6 +57,7 @@
 #include <stdlib.h>
 #include <errno.h>
 #include <ctype.h>
+#include <limits.h>
 #include <openssl/bio.h>
 
 #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 3d7282eabbcce304f2f77b5d0d4c2c9e300f800e..5db78fc216d3b46527077f99cc0664cc994f2ded 100644 (file)
--- 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 150eef4fb15cf4f0ad2134975bec5cc4fb567c8c..bd6fdff123ff447705ae2b9774e26ee1e8ed3edc 100644 (file)
--- 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));