SSL_set1_groups_list(): Fix memory corruption with 40 groups and more

Fixes #23624

The calculation of the size for gid_arr reallocation was wrong.
A multiplication by gid_arr array item size was missing.

Testcase is added.

Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23625)
This commit is contained in:
Michael Baentsch 2024-02-19 06:41:35 +01:00 committed by Tomas Mraz
parent db511578f7
commit f4ed6eed2c
3 changed files with 11 additions and 14 deletions

View File

@ -1049,7 +1049,8 @@ static int gid_cb(const char *elem, int len, void *arg)
return 0;
if (garg->gidcnt == garg->gidmax) {
uint16_t *tmp =
OPENSSL_realloc(garg->gid_arr, garg->gidmax + GROUPLIST_INCREMENT);
OPENSSL_realloc(garg->gid_arr,
(garg->gidmax + GROUPLIST_INCREMENT) * sizeof(*garg->gid_arr));
if (tmp == NULL)
return 0;
garg->gidmax += GROUPLIST_INCREMENT;

View File

@ -9652,20 +9652,11 @@ static int test_pluggable_group(int idx)
OSSL_PROVIDER *tlsprov = OSSL_PROVIDER_load(libctx, "tls-provider");
/* Check that we are not impacted by a provider without any groups */
OSSL_PROVIDER *legacyprov = OSSL_PROVIDER_load(libctx, "legacy");
const char *group_name = idx == 0 ? "xorgroup" : "xorkemgroup";
const char *group_name = idx == 0 ? "xorkemgroup" : "xorgroup";
if (!TEST_ptr(tlsprov))
goto end;
if (legacyprov == NULL) {
/*
* In this case we assume we've been built with "no-legacy" and skip
* this test (there is no OPENSSL_NO_LEGACY)
*/
testresult = 1;
goto end;
}
if (!TEST_true(create_ssl_ctx_pair(libctx, TLS_server_method(),
TLS_client_method(),
TLS1_3_VERSION,
@ -9675,7 +9666,9 @@ static int test_pluggable_group(int idx)
NULL, NULL)))
goto end;
if (!TEST_true(SSL_set1_groups_list(serverssl, group_name))
/* ensure GROUPLIST_INCREMENT (=40) logic triggers: */
if (!TEST_true(SSL_set1_groups_list(serverssl, "xorgroup:xorkemgroup:dummy1:dummy2:dummy3:dummy4:dummy5:dummy6:dummy7:dummy8:dummy9:dummy10:dummy11:dummy12:dummy13:dummy14:dummy15:dummy16:dummy17:dummy18:dummy19:dummy20:dummy21:dummy22:dummy23:dummy24:dummy25:dummy26:dummy27:dummy28:dummy29:dummy30:dummy31:dummy32:dummy33:dummy34:dummy35:dummy36:dummy37:dummy38:dummy39:dummy40:dummy41:dummy42:dummy43"))
/* removing a single algorithm from the list makes the test pass */
|| !TEST_true(SSL_set1_groups_list(clientssl, group_name)))
goto end;

View File

@ -410,6 +410,8 @@ static int tls_prov_get_capabilities(void *provctx, const char *capability,
}
dummygroup[0].data = dummy_group_names[i];
dummygroup[0].data_size = strlen(dummy_group_names[i]) + 1;
/* assign unique group IDs also to dummy groups for registration */
*((int *)(dummygroup[3].data)) = 65279 - NUM_DUMMY_GROUPS + i;
ret &= cb(dummygroup, arg);
}
}
@ -3185,9 +3187,10 @@ unsigned int randomize_tls_alg_id(OSSL_LIB_CTX *libctx)
return 0;
/*
* Ensure id is within the IANA Reserved for private use range
* (65024-65279)
* (65024-65279).
* Carve out NUM_DUMMY_GROUPS ids for properly registering those.
*/
id %= 65279 - 65024;
id %= 65279 - NUM_DUMMY_GROUPS - 65024;
id += 65024;
/* Ensure we did not already issue this id */