From cad8347be23c5e0c0d9eea02d090d42daf2dd7a9 Mon Sep 17 00:00:00 2001 From: Shane Lontis Date: Wed, 27 Mar 2019 17:38:28 +1000 Subject: [PATCH] fixed public range check in ec_GF2m_simple_oct2point Reviewed-by: Matt Caswell Reviewed-by: Bernd Edlinger (Merged from https://github.com/openssl/openssl/pull/8607) --- crypto/ec/ec2_oct.c | 9 ++++--- test/evp_extra_test.c | 62 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/crypto/ec/ec2_oct.c b/crypto/ec/ec2_oct.c index 59258392a2..ca36bb1673 100644 --- a/crypto/ec/ec2_oct.c +++ b/crypto/ec/ec2_oct.c @@ -237,7 +237,7 @@ int ec_GF2m_simple_oct2point(const EC_GROUP *group, EC_POINT *point, BN_CTX *ctx) { point_conversion_form_t form; - int y_bit; + int y_bit, m; BN_CTX *new_ctx = NULL; BIGNUM *x, *y, *yxi; size_t field_len, enc_len; @@ -270,7 +270,8 @@ int ec_GF2m_simple_oct2point(const EC_GROUP *group, EC_POINT *point, return EC_POINT_set_to_infinity(group, point); } - field_len = (EC_GROUP_get_degree(group) + 7) / 8; + m = EC_GROUP_get_degree(group); + field_len = (m + 7) / 8; enc_len = (form == POINT_CONVERSION_COMPRESSED) ? 1 + field_len : 1 + 2 * field_len; @@ -295,7 +296,7 @@ int ec_GF2m_simple_oct2point(const EC_GROUP *group, EC_POINT *point, if (!BN_bin2bn(buf + 1, field_len, x)) goto err; - if (BN_ucmp(x, group->field) >= 0) { + if (BN_num_bits(x) > m) { ECerr(EC_F_EC_GF2M_SIMPLE_OCT2POINT, EC_R_INVALID_ENCODING); goto err; } @@ -306,7 +307,7 @@ int ec_GF2m_simple_oct2point(const EC_GROUP *group, EC_POINT *point, } else { if (!BN_bin2bn(buf + 1 + field_len, field_len, y)) goto err; - if (BN_ucmp(y, group->field) >= 0) { + if (BN_num_bits(y) > m) { ECerr(EC_F_EC_GF2M_SIMPLE_OCT2POINT, EC_R_INVALID_ENCODING); goto err; } diff --git a/test/evp_extra_test.c b/test/evp_extra_test.c index f68b013844..f07ae94ad3 100644 --- a/test/evp_extra_test.c +++ b/test/evp_extra_test.c @@ -510,6 +510,66 @@ static int test_d2i_AutoPrivateKey(int i) } #ifndef OPENSSL_NO_EC + +static const unsigned char ec_public_sect163k1_validxy[] = { + 0x30, 0x40, 0x30, 0x10, 0x06, 0x07, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, + 0x01, 0x06, 0x05, 0x2b, 0x81, 0x04, 0x00, 0x01, 0x03, 0x2c, 0x00, 0x04, + 0x02, 0x84, 0x58, 0xa6, 0xd4, 0xa0, 0x35, 0x2b, 0xae, 0xf0, 0xc0, 0x69, + 0x05, 0xcf, 0x2a, 0x50, 0x33, 0xf9, 0xe3, 0x92, 0x79, 0x02, 0xd1, 0x7b, + 0x9f, 0x22, 0x00, 0xf0, 0x3b, 0x0e, 0x5d, 0x2e, 0xb7, 0x23, 0x24, 0xf3, + 0x6a, 0xd8, 0x17, 0x65, 0x41, 0x2f +}; + +static const unsigned char ec_public_sect163k1_badx[] = { + 0x30, 0x40, 0x30, 0x10, 0x06, 0x07, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, + 0x01, 0x06, 0x05, 0x2b, 0x81, 0x04, 0x00, 0x01, 0x03, 0x2c, 0x00, 0x04, + 0x0a, 0x84, 0x58, 0xa6, 0xd4, 0xa0, 0x35, 0x2b, 0xae, 0xf0, 0xc0, 0x69, + 0x05, 0xcf, 0x2a, 0x50, 0x33, 0xf9, 0xe3, 0x92, 0xb0, 0x02, 0xd1, 0x7b, + 0x9f, 0x22, 0x00, 0xf0, 0x3b, 0x0e, 0x5d, 0x2e, 0xb7, 0x23, 0x24, 0xf3, + 0x6a, 0xd8, 0x17, 0x65, 0x41, 0x2f +}; + +static const unsigned char ec_public_sect163k1_bady[] = { + 0x30, 0x40, 0x30, 0x10, 0x06, 0x07, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, + 0x01, 0x06, 0x05, 0x2b, 0x81, 0x04, 0x00, 0x01, 0x03, 0x2c, 0x00, 0x04, + 0x02, 0x84, 0x58, 0xa6, 0xd4, 0xa0, 0x35, 0x2b, 0xae, 0xf0, 0xc0, 0x69, + 0x05, 0xcf, 0x2a, 0x50, 0x33, 0xf9, 0xe3, 0x92, 0x79, 0x0a, 0xd1, 0x7b, + 0x9f, 0x22, 0x00, 0xf0, 0x3b, 0x0e, 0x5d, 0x2e, 0xb7, 0x23, 0x24, 0xf3, + 0x6a, 0xd8, 0x17, 0x65, 0x41, 0xe6 +}; + +static struct ec_der_pub_keys_st { + const unsigned char *der; + size_t len; + int valid; +} ec_der_pub_keys[] = { + { ec_public_sect163k1_validxy, sizeof(ec_public_sect163k1_validxy), 1 }, + { ec_public_sect163k1_badx, sizeof(ec_public_sect163k1_badx), 0 }, + { ec_public_sect163k1_bady, sizeof(ec_public_sect163k1_bady), 0 }, +}; + +/* + * Tests the range of the decoded EC char2 public point. + * See ec_GF2m_simple_oct2point(). + */ +static int test_invalide_ec_char2_pub_range_decode(int id) +{ + int ret = 0; + BIO *bio = NULL; + EC_KEY *eckey = NULL; + + if (!TEST_ptr(bio = BIO_new_mem_buf(ec_der_pub_keys[id].der, + ec_der_pub_keys[id].len))) + goto err; + eckey = d2i_EC_PUBKEY_bio(bio, NULL); + ret = (ec_der_pub_keys[id].valid && TEST_ptr(eckey)) + || TEST_ptr_null(eckey); +err: + EC_KEY_free(eckey); + BIO_free(bio); + return ret; +} + /* Tests loading a bad key in PKCS8 format */ static int test_EVP_PKCS82PKEY(void) { @@ -1136,6 +1196,8 @@ int setup_tests(void) ADD_TEST(test_HKDF); #ifndef OPENSSL_NO_EC ADD_TEST(test_X509_PUBKEY_inplace); + ADD_ALL_TESTS(test_invalide_ec_char2_pub_range_decode, + OSSL_NELEM(ec_der_pub_keys)); #endif ADD_ALL_TESTS(test_EVP_MD_fetch, 3); return 1; -- 2.25.1