Avoid potential OOB if width > sizeof(start)

This can't currently happen due to sizeof(start) being way larger than MAX_OPT_HELP_WIDTH,
but wasn't checked for previously. With this patch there still remains one (static) OOB,
when the length of the option name and the valtype2param string for that argument overflow
the buffer in opt_print. This is kinda unlikely, unless someone intentionally crafts a
long option name, in which case this would become some trivial stack buffer overrun with
possibility to overwrite pointer to the OPTIONS structure (a long o->name is critical here).

I sincerely hope we trust our built-in documentation to not exploit ourselves.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/12265)

diff --git a/apps/lib/opt.c b/apps/lib/opt.c
index 73e29486815cfd988be3111d2d7c04af660220a0..ce81408e38074845c0203fc9bf8d8a1a877946c3 100644 (file)
--- a/apps/lib/opt.c
+++ b/apps/lib/opt.c
@@ -1111,53 +1111,62 @@ static void opt_print(const OPTIONS *o, int doingparams, int width)
     char start[80 + 1];
     char *p;
 
-        help = o->helpstr ? o->helpstr : "(No additional info)";
-        if (o->name == OPT_HELP_STR) {
-            opt_printf_stderr(help, prog);
-            return;
-        }
-        if (o->name == OPT_SECTION_STR) {
-            opt_printf_stderr("\n");
-            opt_printf_stderr(help, prog);
-            return;
-        }
-        if (o->name == OPT_PARAM_STR) {
-            opt_printf_stderr("\nParameters:\n");
-            return;
-        }
-
-        /* Pad out prefix */
-        memset(start, ' ', sizeof(start) - 1);
-        start[sizeof(start) - 1] = '\0';
+    /* Avoid OOB if width is beyond the buffer size of start */
+    if (width >= (int)sizeof(start))
+        width = (int)sizeof(start) - 1;
+
+    help = o->helpstr ? o->helpstr : "(No additional info)";
+    if (o->name == OPT_HELP_STR) {
+        opt_printf_stderr(help, prog);
+        return;
+    } else if (o->name == OPT_SECTION_STR) {
+        opt_printf_stderr("\n");
+        opt_printf_stderr(help, prog);
+        return;
+    } else if (o->name == OPT_PARAM_STR) {
+        opt_printf_stderr("\nParameters:\n");
+        return;
+    }
 
-        if (o->name == OPT_MORE_STR) {
-            /* Continuation of previous line; pad and print. */
-            start[width] = '\0';
-            opt_printf_stderr("%s  %s\n", start, help);
-            return;
-        }
+    /* Pad out prefix */
+    memset(start, ' ', sizeof(start) - 1);
+    start[sizeof(start) - 1] = '\0';
 
-        /* Build up the "-flag [param]" part. */
-        p = start;
-        *p++ = ' ';
-        if (!doingparams)
-            *p++ = '-';
-        if (o->name[0])
-            p += strlen(strcpy(p, o->name));
-        else
-            *p++ = '*';
-        if (o->valtype != '-') {
-            *p++ = ' ';
-            p += strlen(strcpy(p, valtype2param(o)));
-        }
-        *p = ' ';
-        if ((int)(p - start) >= MAX_OPT_HELP_WIDTH) {
-            *p = '\0';
-            opt_printf_stderr("%s\n", start);
-            memset(start, ' ', sizeof(start));
-        }
+    if (o->name == OPT_MORE_STR) {
+        /* Continuation of previous line; pad and print. */
         start[width] = '\0';
         opt_printf_stderr("%s  %s\n", start, help);
+        return;
+    }
+
+    /* Build up the "-flag [param]" part. */
+    p = start;
+
+    *p++ = ' ';
+
+    if (!doingparams)
+        *p++ = '-';
+
+    if (o->name[0])
+        p += strlen(strcpy(p, o->name));
+    else
+        *p++ = '*';
+
+    if (o->valtype != '-') {
+        *p++ = ' ';
+        p += strlen(strcpy(p, valtype2param(o)));
+    }
+
+    *p = ' ';
+
+    if ((int)(p - start) >= MAX_OPT_HELP_WIDTH) {
+        *p = '\0';
+        opt_printf_stderr("%s\n", start);
+        memset(start, ' ', sizeof(start));
+    }
+
+    start[width] = '\0';
+    opt_printf_stderr("%s  %s\n", start, help);
 }
 
 void opt_help(const OPTIONS *list)
@@ -1165,7 +1174,6 @@ void opt_help(const OPTIONS *list)
     const OPTIONS *o;
     int i, sawparams = 0, width = 5;
     int standard_prolog;
-    char start[80 + 1];
 
     /* Starts with its own help message? */
     standard_prolog = list[0].name != OPT_HELP_STR;
@@ -1174,14 +1182,18 @@ void opt_help(const OPTIONS *list)
     for (o = list; o->name; o++) {
         if (o->name == OPT_MORE_STR)
             continue;
+
         i = 2 + (int)strlen(o->name);
         if (o->valtype != '-')
             i += 1 + strlen(valtype2param(o));
-        if (i < MAX_OPT_HELP_WIDTH && i > width)
+
+        if (i > width)
             width = i;
-        OPENSSL_assert(i < (int)sizeof(start));
     }
 
+    if (width > MAX_OPT_HELP_WIDTH)
+        width = MAX_OPT_HELP_WIDTH;
+
     if (standard_prolog) {
         opt_printf_stderr("Usage: %s [options]\n", prog);
         if (list[0].name != OPT_SECTION_STR)