From: tb <tb@openbsd.org>
Date: Tue, 14 Aug 2018 17:51:36 +0000 (+0000)
Subject: The UI_add_{input,verify}_string() functions want a length not including
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=ddcb65f9746809d209494b262e6a032f4cb86bfe;p=openbsd

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 33de513ef29..debd1b75610 100644
--- 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);