The UI_add_{input,verify}_string() functions want a length not including

the terminating NUL. EVP_read_pw_string_min() got this wrong, leading to
a one-byte buffer overrun in all callers of EVP_read_pw_string().

Found by mestre running 'openssl passwd' with MALLOC_OPTIONS including C.

Fix this by doing some basic sanity checking in EVP_read_pw_string_min().
Cap the len argument at BUFSIZ and ensure that min < len as well as
0 <= min and 1 <= len.  The last two checks are important as these
numbers may end up in reallocarray().

ok bcook (on previous version), jsing, mestre

diff --git a/lib/libcrypto/evp/evp_key.c b/lib/libcrypto/evp/evp_key.c
index 33de513..debd1b7 100644 (file)
--- a/lib/libcrypto/evp/evp_key.c
+++ b/lib/libcrypto/evp/evp_key.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: evp_key.c,v 1.24 2017/01/29 17:49:23 beck Exp $ */
+/* $OpenBSD: evp_key.c,v 1.25 2018/08/14 17:51:36 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -101,17 +101,20 @@ EVP_read_pw_string_min(char *buf, int min, int len, const char *prompt,
        char buff[BUFSIZ];
        UI *ui;
 
+       if (len > BUFSIZ)
+               len = BUFSIZ;
+       if (min < 0 || len - 1 < min)
+               return -1;
        if ((prompt == NULL) && (prompt_string[0] != '\0'))
                prompt = prompt_string;
        ui = UI_new();
        if (ui == NULL)
                return -1;
-       if (UI_add_input_string(ui, prompt, 0, buf, min,
-           (len >= BUFSIZ) ? BUFSIZ - 1 : len) < 0)
+       if (UI_add_input_string(ui, prompt, 0, buf, min, len - 1) < 0)
                return -1;
        if (verify) {
-               if (UI_add_verify_string(ui, prompt, 0, buff, min,
-                   (len >= BUFSIZ) ? BUFSIZ - 1 : len, buf) < 0)
+               if (UI_add_verify_string(ui, prompt, 0, buff, min, len - 1, buf)
+                   < 0)
                        return -1;
        }
        ret = UI_process(ui);