From 986c48c4eb26861f25bc68ea252d8f2aad592735 Mon Sep 17 00:00:00 2001
From: Neil Horman <nhorman@openssl.org>
Date: Tue, 28 Nov 2023 13:54:37 -0500
Subject: [PATCH] Add overflow checks to parse_number/parse_hex/parse_oct

Test the next arithmetic operation to safely determine if adding the
next digit in the passed property string will overflow

Also, noted a bug in the parse_hex code.  When parsing non-digit
characters (i.e. a-f and A-F), we do a tolower conversion (which is
fine), and then subtract 'a' to get the hex value from the ascii (which
is definately wrong).  We should subtract 'W' to convert tolower
converted hex digits in the range a-f to their hex value counterparts

Add tests to test_property_parse_error to ensure overflow checks work

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
(Merged from https://github.com/openssl/openssl/pull/22874)
---
 crypto/property/property_parse.c | 50 +++++++++++++++++++++++++-------
 test/property_test.c             |  7 +++++
 2 files changed, 47 insertions(+), 10 deletions(-)

diff --git a/crypto/property/property_parse.c b/crypto/property/property_parse.c
index 983f07e070..9820916ab4 100644
--- a/crypto/property/property_parse.c
+++ b/crypto/property/property_parse.c
@@ -97,9 +97,18 @@ static int parse_number(const char *t[], OSSL_PROPERTY_DEFINITION *res)
     const char *s = *t;
     int64_t v = 0;
 
-    if (!ossl_isdigit(*s))
-        return 0;
     do {
+        if (!ossl_isdigit(*s)) {
+            ERR_raise_data(ERR_LIB_PROP, PROP_R_NOT_A_DECIMAL_DIGIT,
+                           "HERE-->%s", *t);
+            return 0;
+        }
+        /* overflow check */
+        if (v > ((INT64_MAX - (*s - '0')) / 10)) {
+            ERR_raise_data(ERR_LIB_PROP, PROP_R_PARSE_FAILED,
+                           "Property %s overflows", *t);
+            return 0;
+        }
         v = v * 10 + (*s++ - '0');
     } while (ossl_isdigit(*s));
     if (!ossl_isspace(*s) && *s != '\0' && *s != ',') {
@@ -117,15 +126,27 @@ static int parse_hex(const char *t[], OSSL_PROPERTY_DEFINITION *res)
 {
     const char *s = *t;
     int64_t v = 0;
+    int sval;
 
-    if (!ossl_isxdigit(*s))
-        return 0;
     do {
+        if (ossl_isdigit(*s)) {
+            sval = *s - '0';
+        } else if (ossl_isxdigit(*s)) {
+            sval = ossl_tolower(*s) - 'a' + 10;
+        } else {
+            ERR_raise_data(ERR_LIB_PROP, PROP_R_NOT_AN_HEXADECIMAL_DIGIT,
+                           "%s", *t);
+            return 0;
+        }
+
+        if (v > ((INT64_MAX - sval) / 16)) {
+            ERR_raise_data(ERR_LIB_PROP, PROP_R_PARSE_FAILED,
+                           "Property %s overflows", *t);
+            return 0;
+        }
+
         v <<= 4;
-        if (ossl_isdigit(*s))
-            v += *s - '0';
-        else
-            v += ossl_tolower(*s) - 'a';
+        v += sval;
     } while (ossl_isxdigit(*++s));
     if (!ossl_isspace(*s) && *s != '\0' && *s != ',') {
         ERR_raise_data(ERR_LIB_PROP, PROP_R_NOT_AN_HEXADECIMAL_DIGIT,
@@ -143,9 +164,18 @@ static int parse_oct(const char *t[], OSSL_PROPERTY_DEFINITION *res)
     const char *s = *t;
     int64_t v = 0;
 
-    if (*s == '9' || *s == '8' || !ossl_isdigit(*s))
-        return 0;
     do {
+        if (*s == '9' || *s == '8' || !ossl_isdigit(*s)) {
+            ERR_raise_data(ERR_LIB_PROP, PROP_R_NOT_AN_OCTAL_DIGIT,
+                           "HERE-->%s", *t);
+            return 0;
+        }
+        if (v > ((INT64_MAX - (*s - '0')) / 8)) {
+            ERR_raise_data(ERR_LIB_PROP, PROP_R_PARSE_FAILED,
+                           "Property %s overflows", *t);
+            return 0;
+        }
+
         v = (v << 3) + (*s - '0');
     } while (ossl_isdigit(*++s) && *s != '9' && *s != '8');
     if (!ossl_isspace(*s) && *s != '\0' && *s != ',') {
diff --git a/test/property_test.c b/test/property_test.c
index bba96fac0a..18f8cc8740 100644
--- a/test/property_test.c
+++ b/test/property_test.c
@@ -136,6 +136,10 @@ static const struct {
     { "n=0x3", "n=3", 1 },
     { "n=0x3", "n=-3", -1 },
     { "n=0x33", "n=51", 1 },
+    { "n=0x123456789abcdef", "n=0x123456789abcdef", 1 },
+    { "n=0x7fffffffffffffff", "n=0x7fffffffffffffff", 1 },   /* INT64_MAX */
+    { "n=9223372036854775807", "n=9223372036854775807", 1 }, /* INT64_MAX */
+    { "n=0777777777777777777777", "n=0777777777777777777777", 1 }, /* INT64_MAX */
     { "n=033", "n=27", 1 },
     { "n=0", "n=00", 1 },
     { "n=0x0", "n=0", 1 },
@@ -198,6 +202,9 @@ static const struct {
     { 1, "a=2, n=012345678" },  /* Bad octal digit */
     { 0, "n=0x28FG, a=3" },     /* Bad hex digit */
     { 0, "n=145d, a=2" },       /* Bad decimal digit */
+    { 0, "n=0x8000000000000000, a=3" },     /* Hex overflow */
+    { 0, "n=922337203000000000d, a=2" },    /* Decimal overflow */
+    { 0, "a=2, n=1000000000000000000000" }, /* Octal overflow */
     { 1, "@='hello'" },         /* Invalid name */
     { 1, "n0123456789012345678901234567890123456789"
          "0123456789012345678901234567890123456789"