From ebd24b37eccf8eb362ab7c5257b57f833eb2a873 Mon Sep 17 00:00:00 2001
From: Richard Levitte <levitte@openssl.org>
Date: Tue, 23 Jan 2024 13:17:31 +0100
Subject: [PATCH] Have OSSL_PARAM_allocate_from_text() fail on odd number of
 hex digits

The failure would be caught later on, so this went unnoticed, until someone
tried with just one hex digit, which was simply ignored.

Fixes #23373

Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23374)

(cherry picked from commit ea6268cfceaba24328d66bd14bfc97c4fac14a58)
---
 crypto/params_from_text.c |  8 ++++++-
 test/params_test.c        | 44 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/crypto/params_from_text.c b/crypto/params_from_text.c
index 360f8933e1355..3fe65af3c51fa 100644
--- a/crypto/params_from_text.c
+++ b/crypto/params_from_text.c
@@ -118,7 +118,13 @@ static int prepare_from_text(const OSSL_PARAM *paramdefs, const char *key,
         break;
     case OSSL_PARAM_OCTET_STRING:
         if (*ishex) {
-            *buf_n = strlen(value) >> 1;
+            size_t hexdigits = strlen(value);
+            if ((hexdigits % 2) != 0) {
+                /* We don't accept an odd number of hex digits */
+                ERR_raise(ERR_LIB_CRYPTO, CRYPTO_R_ODD_NUMBER_OF_DIGITS);
+                return 0;
+            }
+            *buf_n = hexdigits >> 1;
         } else {
             *buf_n = value_n;
         }
diff --git a/test/params_test.c b/test/params_test.c
index 6a970feaa4591..3df1669812df3 100644
--- a/test/params_test.c
+++ b/test/params_test.c
@@ -15,6 +15,7 @@
 #include <string.h>
 #include <openssl/bn.h>
 #include <openssl/core.h>
+#include <openssl/err.h>
 #include <openssl/params.h>
 #include "internal/numbers.h"
 #include "internal/nelem.h"
@@ -558,6 +559,7 @@ static const OSSL_PARAM params_from_text[] = {
     /* Arbitrary size buffer.  Make sure the result fits in a long */
     OSSL_PARAM_DEFN("num", OSSL_PARAM_INTEGER, NULL, 0),
     OSSL_PARAM_DEFN("unum", OSSL_PARAM_UNSIGNED_INTEGER, NULL, 0),
+    OSSL_PARAM_DEFN("octets", OSSL_PARAM_OCTET_STRING, NULL, 0),
     OSSL_PARAM_END,
 };
 
@@ -655,14 +657,56 @@ static int check_int_from_text(const struct int_from_text_test_st a)
     return a.expected_res;
 }
 
+static int check_octetstr_from_hexstr(void)
+{
+    OSSL_PARAM param;
+    static const char *values[] = { "", "F", "FF", "FFF", "FFFF", NULL };
+    int i;
+    int errcnt = 0;
+
+    /* Test odd vs even number of hex digits */
+    for (i = 0; values[i] != NULL; i++) {
+        int expected = (strlen(values[i]) % 2) != 1;
+        int result;
+
+        ERR_clear_error();
+        memset(&param, 0, sizeof(param));
+        if (expected)
+            result =
+                TEST_true(OSSL_PARAM_allocate_from_text(&param,
+                                                        params_from_text,
+                                                        "hexoctets", values[i], 0,
+                                                        NULL));
+        else
+            result =
+                TEST_false(OSSL_PARAM_allocate_from_text(&param,
+                                                         params_from_text,
+                                                         "hexoctets", values[i], 0,
+                                                         NULL));
+        if (!result) {
+            TEST_error("unexpected OSSL_PARAM_allocate_from_text() %s for 'octets' \"%s\"",
+                       (expected ? "failure" : "success"), values[i]);
+            errcnt++;
+        }
+        OPENSSL_free(param.data);
+    }
+    return errcnt == 0;
+}
+
 static int test_allocate_from_text(int i)
 {
     return check_int_from_text(int_from_text_test_cases[i]);
 }
 
+static int test_more_allocate_from_text(void)
+{
+    return check_octetstr_from_hexstr();
+}
+
 int setup_tests(void)
 {
     ADD_ALL_TESTS(test_case, OSSL_NELEM(test_cases));
     ADD_ALL_TESTS(test_allocate_from_text, OSSL_NELEM(int_from_text_test_cases));
+    ADD_TEST(test_more_allocate_from_text);
     return 1;
 }