NCONF_get_number refix.

Fix the NULL check lack in a different way that is more compatible with
non-NULL branch.  Refer #6632

Also mark and pop the error stack instead of clearing all errors when something
goes awry in CONF_get_number.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6643)

5 files changed, 61 insertions, 12 deletions

diff --git a/crypto/conf/conf_err.c b/crypto/conf/conf_err.c
index 01f98da..f761358 100644
--- a/crypto/conf/conf_err.c
+++ b/crypto/conf/conf_err.c
@@ -60,6 +60,7 @@ static const ERR_STRING_DATA CONF_str_reasons[] = {
     {ERR_PACK(ERR_LIB_CONF, 0, CONF_R_NO_SECTION), "no section"},
     {ERR_PACK(ERR_LIB_CONF, 0, CONF_R_NO_SUCH_FILE), "no such file"},
     {ERR_PACK(ERR_LIB_CONF, 0, CONF_R_NO_VALUE), "no value"},
+    {ERR_PACK(ERR_LIB_CONF, 0, CONF_R_NUMBER_TOO_LARGE), "number too large"},
     {ERR_PACK(ERR_LIB_CONF, 0, CONF_R_RECURSIVE_DIRECTORY_INCLUDE),
     "recursive directory include"},
     {ERR_PACK(ERR_LIB_CONF, 0, CONF_R_SSL_COMMAND_SECTION_EMPTY),
diff --git a/crypto/conf/conf_lib.c b/crypto/conf/conf_lib.c
index 5f976f3..1833b15 100644
--- a/crypto/conf/conf_lib.c
+++ b/crypto/conf/conf_lib.c
@@ -11,6 +11,7 @@
 #include <stdio.h>
 #include <string.h>
 #include "internal/conf.h"
+#include "internal/ctype.h"
 #include <openssl/crypto.h>
 #include <openssl/err.h>
 #include <openssl/conf.h>
@@ -123,6 +124,7 @@ long CONF_get_number(LHASH_OF(CONF_VALUE) *conf, const char *group,
     int status;
     long result = 0;
 
+    ERR_set_mark();
     if (conf == NULL) {
         status = NCONF_get_number_e(NULL, group, name, &result);
     } else {
@@ -130,12 +132,8 @@ long CONF_get_number(LHASH_OF(CONF_VALUE) *conf, const char *group,
         CONF_set_nconf(&ctmp, conf);
         status = NCONF_get_number_e(&ctmp, group, name, &result);
     }
-
-    if (status == 0) {
-        /* This function does not believe in errors... */
-        ERR_clear_error();
-    }
-    return result;
+    ERR_pop_to_mark();
+    return status == 0 ? 0L : result;
 }
 
 void CONF_free(LHASH_OF(CONF_VALUE) *conf)
@@ -277,10 +275,23 @@ char *NCONF_get_string(const CONF *conf, const char *group, const char *name)
     return NULL;
 }
 
+static int default_is_number(const CONF *conf, char c)
+{
+    return ossl_isdigit(c);
+}
+
+static int default_to_int(const CONF *conf, char c)
+{
+    return (int)(c - '0');
+}
+
 int NCONF_get_number_e(const CONF *conf, const char *group, const char *name,
                        long *result)
 {
     char *str;
+    long res;
+    int (*is_number)(const CONF *, char) = &default_is_number;
+    int (*to_int)(const CONF *, char) = &default_to_int;
 
     if (result == NULL) {
         CONFerr(CONF_F_NCONF_GET_NUMBER_E, ERR_R_PASSED_NULL_PARAMETER);
@@ -292,14 +303,23 @@ int NCONF_get_number_e(const CONF *conf, const char *group, const char *name,
     if (str == NULL)
         return 0;
 
-    if (conf == NULL)
-        *result = strtol(str, &str, 10);
-    else
-        for (*result = 0; conf->meth->is_number(conf, *str);) {
-            *result = (*result) * 10 + conf->meth->to_int(conf, *str);
-            str++;
+    if (conf != NULL) {
+        if (conf->meth->is_number != NULL)
+            is_number = conf->meth->is_number;
+        if (conf->meth->to_int != NULL)
+            to_int = conf->meth->to_int;
+    }
+    for (res = 0; is_number(conf, *str); str++) {
+        const int d = to_int(conf, *str);
+
+        if (res > (LONG_MAX - d) / 10L) {
+            CONFerr(CONF_F_NCONF_GET_NUMBER_E, CONF_R_NUMBER_TOO_LARGE);
+            return 0;
         }
+        res = res * 10 + d;
+    }
 
+    *result = res;
     return 1;
 }
 
diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
index ee68388..007560a 100644
--- a/crypto/err/openssl.txt
+++ b/crypto/err/openssl.txt
@@ -2012,6 +2012,7 @@ CONF_R_NO_CONF_OR_ENVIRONMENT_VARIABLE:106:no conf or environment variable
 CONF_R_NO_SECTION:107:no section
 CONF_R_NO_SUCH_FILE:114:no such file
 CONF_R_NO_VALUE:108:no value
+CONF_R_NUMBER_TOO_LARGE:121:number too large
 CONF_R_RECURSIVE_DIRECTORY_INCLUDE:111:recursive directory include
 CONF_R_SSL_COMMAND_SECTION_EMPTY:117:ssl command section empty
 CONF_R_SSL_COMMAND_SECTION_NOT_FOUND:118:ssl command section not found
diff --git a/include/openssl/conferr.h b/include/openssl/conferr.h
index 4574636..d1c92f4 100644
--- a/include/openssl/conferr.h
+++ b/include/openssl/conferr.h
@@ -58,6 +58,7 @@ int ERR_load_CONF_strings(void);
 # define CONF_R_NO_SECTION                                107
 # define CONF_R_NO_SUCH_FILE                              114
 # define CONF_R_NO_VALUE                                  108
+# define CONF_R_NUMBER_TOO_LARGE                          121
 # define CONF_R_RECURSIVE_DIRECTORY_INCLUDE               111
 # define CONF_R_SSL_COMMAND_SECTION_EMPTY                 117
 # define CONF_R_SSL_COMMAND_SECTION_NOT_FOUND             118
diff --git a/test/conf_include_test.c b/test/conf_include_test.c
index ba79d2c..ee02d9b 100644
--- a/test/conf_include_test.c
+++ b/test/conf_include_test.c
@@ -153,6 +153,31 @@ static int test_check_null_numbers(void)
     return 1;
 }
 
+static int test_check_overflow(void)
+{
+#if defined(_BSD_SOURCE) \
+        || (defined(_POSIX_C_SOURCE) && _POSIX_C_SOURCE >= 200112L) \
+        || (defined(_XOPEN_SOURCE) && _XOPEN_SOURCE >= 600)
+    long val = 0;
+    char max[(sizeof(long) * 8) / 3 + 3];
+    char *p;
+
+    p = max + sprintf(max, "0%ld", LONG_MAX) - 1;
+    setenv("FNORD", max, 1);
+    if (!TEST_true(NCONF_get_number(NULL, "missing", "FNORD", &val))
+            || !TEST_long_eq(val, LONG_MAX))
+        return 0;
+
+    while (++*p > '9')
+        *p-- = '0';
+
+    setenv("FNORD", max, 1);
+    if (!TEST_false(NCONF_get_number(NULL, "missing", "FNORD", &val)))
+        return 0;
+#endif
+    return 1;
+}
+
 int setup_tests(void)
 {
     const char *conf_file;
@@ -181,6 +206,7 @@ int setup_tests(void)
 
     ADD_TEST(test_load_config);
     ADD_TEST(test_check_null_numbers);
+    ADD_TEST(test_check_overflow);
     return 1;
 }