Skip to content

Add bounds checks on OSSL_PARAMS for EC, DH and RSA #398

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
padelsbach wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Add bounds checks on OSSL_PARAMS for EC, DH and RSA #398

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions test/test_dh.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -718,4 +718,53 @@ int test_dh_krb5_keygen(void *data)
return err;
}

/* Pass an oversize OSSL_PARAM (>1024 bytes) for the FFC P component to
* EVP_PKEY_fromdata so wp_mp_read_unsigned_bin_le (wp_params.c) hits its
* size-guard branch via wp_dh_import_group. Expect failure rather than the
* stack buffer overflow that would happen without the guard. */
int test_dh_fromdata_oversize(void *data)
{
int err = 0;
EVP_PKEY_CTX *ctx_wolf = NULL;
EVP_PKEY *pkey_wolf = NULL;
unsigned char p_oversize[2048];
unsigned char g_buf[1] = { 0x02 };
OSSL_PARAM params[3];
int status;

(void)data;
memset(p_oversize, 0xAA, sizeof(p_oversize));

PRINT_MSG("Testing EVP_PKEY_fromdata with oversize DH FFC_P component");

ctx_wolf = EVP_PKEY_CTX_new_from_name(wpLibCtx, "DH", NULL);
if (ctx_wolf == NULL) {
err = 1;
}

if (err == 0) {
err |= EVP_PKEY_fromdata_init(ctx_wolf) != 1;
}

if (err == 0) {
params[0] = OSSL_PARAM_construct_BN(OSSL_PKEY_PARAM_FFC_P,
p_oversize, sizeof(p_oversize));
params[1] = OSSL_PARAM_construct_BN(OSSL_PKEY_PARAM_FFC_G,
g_buf, sizeof(g_buf));
params[2] = OSSL_PARAM_construct_end();

status = EVP_PKEY_fromdata(ctx_wolf, &pkey_wolf,
EVP_PKEY_KEY_PARAMETERS, params);
if (status == 1) {
PRINT_MSG("EVP_PKEY_fromdata unexpectedly succeeded with 2048-byte"
" FFC_P");
err = 1;
}
EVP_PKEY_free(pkey_wolf);
}

EVP_PKEY_CTX_free(ctx_wolf);
return err;
}

#endif /* WP_HAVE_DH */
52 changes: 52 additions & 0 deletions test/test_ecc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2618,6 +2618,58 @@ int test_ec_print_public(void* data)

return err;
}

/* Pass an oversize OSSL_PARAM (>1024 bytes) for the EC private key component
* to EVP_PKEY_fromdata so wp_mp_read_unsigned_bin_le (wp_params.c) hits its
* size-guard branch via wp_params_get_mp in wp_ecc_import_keypair. Expect
* failure rather than the stack buffer overflow that would happen without
* the guard. This path also covers ECDH, whose key import shares
* wp_ecc_kmgmt. */
int test_ec_fromdata_oversize(void *data)
{
int err = 0;
EVP_PKEY_CTX *ctx_wolf = NULL;
EVP_PKEY *pkey_wolf = NULL;
unsigned char priv_oversize[2048];
char group_name[] = "prime256v1";
OSSL_PARAM params[3];
int status;

(void)data;
memset(priv_oversize, 0xAA, sizeof(priv_oversize));

PRINT_MSG("Testing EVP_PKEY_fromdata with oversize EC PRIV_KEY component");

ctx_wolf = EVP_PKEY_CTX_new_from_name(wpLibCtx, "EC", NULL);
if (ctx_wolf == NULL) {
err = 1;
}

if (err == 0) {
err |= EVP_PKEY_fromdata_init(ctx_wolf) != 1;
}

if (err == 0) {
params[0] = OSSL_PARAM_construct_utf8_string(OSSL_PKEY_PARAM_GROUP_NAME,
group_name, 0);
params[1] = OSSL_PARAM_construct_BN(OSSL_PKEY_PARAM_PRIV_KEY,
priv_oversize,
sizeof(priv_oversize));
params[2] = OSSL_PARAM_construct_end();

status = EVP_PKEY_fromdata(ctx_wolf, &pkey_wolf, EVP_PKEY_KEYPAIR,
params);
if (status == 1) {
PRINT_MSG("EVP_PKEY_fromdata unexpectedly succeeded with 2048-byte"
" EC PRIV_KEY");
err = 1;
}
EVP_PKEY_free(pkey_wolf);
}

EVP_PKEY_CTX_free(ctx_wolf);
return err;
}
#endif /* WP_HAVE_EC_P256 */

#endif /* WP_HAVE_ECC */
49 changes: 49 additions & 0 deletions test/test_rsa.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1619,6 +1619,55 @@ int test_rsa_fromdata(void* data)
return err;
}

/* Pass an oversize OSSL_PARAM (>1024 bytes) for the N component to
* EVP_PKEY_fromdata so wp_mp_read_unsigned_bin_le (wp_params.c) hits its
* size-guard branch. Expect failure rather than the stack buffer overflow
* that would happen without the guard. */
int test_rsa_fromdata_oversize(void* data)
{
int err = 0;
EVP_PKEY_CTX *ctx_wolf = NULL;
EVP_PKEY *pkey_wolf = NULL;
unsigned char n_oversize[2048];
unsigned char e_buf[3] = { 0x01, 0x00, 0x01 };
OSSL_PARAM params[3];
int status;

(void)data;
memset(n_oversize, 0xAA, sizeof(n_oversize));

PRINT_MSG("Testing EVP_PKEY_fromdata with oversize RSA n component");

ctx_wolf = EVP_PKEY_CTX_new_from_name(wpLibCtx, "RSA", NULL);
if (ctx_wolf == NULL) {
err = 1;
}

if (err == 0) {
err |= EVP_PKEY_fromdata_init(ctx_wolf) != 1;
}

if (err == 0) {
params[0] = OSSL_PARAM_construct_BN(OSSL_PKEY_PARAM_RSA_N,
n_oversize, sizeof(n_oversize));
params[1] = OSSL_PARAM_construct_BN(OSSL_PKEY_PARAM_RSA_E,
e_buf, sizeof(e_buf));
params[2] = OSSL_PARAM_construct_end();

status = EVP_PKEY_fromdata(ctx_wolf, &pkey_wolf, EVP_PKEY_PUBLIC_KEY,
params);
if (status == 1) {
PRINT_MSG("EVP_PKEY_fromdata unexpectedly succeeded with 2048-byte"
" n");
err = 1;
}
EVP_PKEY_free(pkey_wolf);
}

EVP_PKEY_CTX_free(ctx_wolf);
return err;
}

static int test_rsa_decode_pkcs8_old(void)
{
int err = 0;
Expand Down
3 changes: 3 additions & 0 deletions test/unit.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ TEST_CASE test_case[] = {
TEST_DECL(test_dh_invalid_kdf_strings, NULL),
TEST_DECL(test_dh_decode, NULL),
TEST_DECL(test_dh_krb5_keygen, NULL),
TEST_DECL(test_dh_fromdata_oversize, NULL),
#ifndef WOLFPROV_QUICKTEST
TEST_DECL(test_dh_get_params, NULL),
#endif
Expand All @@ -327,6 +328,7 @@ TEST_CASE test_case[] = {
TEST_DECL(test_rsa_load_key, NULL),
TEST_DECL(test_rsa_load_cert, NULL),
TEST_DECL(test_rsa_fromdata, NULL),
TEST_DECL(test_rsa_fromdata_oversize, NULL),
TEST_DECL(test_rsa_decode_pkcs8, NULL),
TEST_DECL(test_rsa_encode_pkcs8, NULL),
TEST_DECL(test_rsa_null_init, NULL),
Expand Down Expand Up @@ -385,6 +387,7 @@ TEST_CASE test_case[] = {
TEST_DECL(test_ec_auto_derive_pubkey, NULL),
TEST_DECL(test_ec_null_init, NULL),
TEST_DECL(test_ec_print_public, NULL),
TEST_DECL(test_ec_fromdata_oversize, NULL),
#endif
#ifdef WP_HAVE_EC_P384
#ifdef WP_HAVE_ECKEYGEN
Expand Down
3 changes: 3 additions & 0 deletions test/unit.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,7 @@ int test_rsa_pss_restrictions(void *data);
int test_rsa_load_key(void* data);
int test_rsa_load_cert(void* data);
int test_rsa_fromdata(void* data);
int test_rsa_fromdata_oversize(void* data);
int test_rsa_decode_pkcs8(void* data);
int test_rsa_encode_pkcs8(void* data);
int test_rsa_null_init(void* data);
Expand All @@ -308,6 +309,7 @@ int test_dh_invalid_kdf_strings(void *data);
int test_dh_decode(void *data);
int test_dh_get_params(void *data);
int test_dh_krb5_keygen(void *data);
int test_dh_fromdata_oversize(void *data);
#endif /* WP_HAVE_DH */

#ifdef WP_HAVE_ECC
Expand Down Expand Up @@ -442,6 +444,7 @@ int test_ec_auto_derive_pubkey(void* data);
int test_ec_null_init(void* data);
#ifdef WP_HAVE_EC_P256
int test_ec_print_public(void* data);
int test_ec_fromdata_oversize(void* data);
#endif

#endif /* WP_HAVE_ECC */
Expand Down
Loading