Skip to content

Commit

Permalink
Have OSSL_PARAM_allocate_from_text() fail on odd number of hex digits
Browse files Browse the repository at this point in the history
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 #23374)
  • Loading branch information
levitte authored and t8m committed Jan 25, 2024
1 parent 8a85df7 commit ea6268c
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 1 deletion.
8 changes: 7 additions & 1 deletion crypto/params_from_text.c
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,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;
}
Expand Down
44 changes: 44 additions & 0 deletions test/params_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
};

Expand Down Expand Up @@ -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;
}

0 comments on commit ea6268c

Please sign in to comment.