From b42b50668036f41597ed62b3c7ab0563a27a4472 Mon Sep 17 00:00:00 2001 From: lanming1120 Date: Thu, 17 Oct 2024 15:10:22 +0800 Subject: [PATCH] Harden BN_GF2m_poly2arr against misuse. Fix CVE-2024-9143. Signed-off-by: lanming1120 --- Configure | 2 +- crypto/bn/bn_gf2m.c | 28 +++++++++++++++------- test/ec_internal_test.c | 51 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 9 deletions(-) diff --git a/Configure b/Configure index 972cca7993..a6b74bf06d 100755 --- a/Configure +++ b/Configure @@ -487,7 +487,7 @@ my @disable_cascades = ( "ssl3-method" => [ "ssl3" ], "zlib" => [ "zlib-dynamic" ], "des" => [ "mdc2" ], - "ec" => [ "ecdsa", "ecdh" ], + "ec" => [ "ec2m", "ecdsa", "ecdh" ], "dgram" => [ "dtls", "sctp" ], "sock" => [ "dgram" ], diff --git a/crypto/bn/bn_gf2m.c b/crypto/bn/bn_gf2m.c index a2ea867551..024eeb6058 100644 --- a/crypto/bn/bn_gf2m.c +++ b/crypto/bn/bn_gf2m.c @@ -15,6 +15,7 @@ #include "bn_local.h" #ifndef OPENSSL_NO_EC2M +# include /* * Maximum number of iterations before BN_GF2m_mod_solve_quad_arr should @@ -1109,16 +1110,26 @@ int BN_GF2m_mod_solve_quad(BIGNUM *r, const BIGNUM *a, const BIGNUM *p, /* * Convert the bit-string representation of a polynomial ( \sum_{i=0}^n a_i * * x^i) into an array of integers corresponding to the bits with non-zero - * coefficient. Array is terminated with -1. Up to max elements of the array - * will be filled. Return value is total number of array elements that would - * be filled if array was large enough. + * coefficient. The array is intended to be suitable for use with + * `BN_GF2m_mod_arr()`, and so the constant term of the polynomial must not be + * zero. This translates to a requirement that the input BIGNUM `a` is odd. + * + * Given sufficient room, the array is terminated with -1. Up to max elements + * of the array will be filled. + * + * The return value is total number of array elements that would be filled if + * array was large enough, including the terminating `-1`. It is `0` when `a` + * is not odd or the constant term is zero contrary to requirement. + * + * The return value is also `0` when the leading exponent exceeds + * `OPENSSL_ECC_MAX_FIELD_BITS`, this guards against CPU exhaustion attacks, */ int BN_GF2m_poly2arr(const BIGNUM *a, int p[], int max) { int i, j, k = 0; BN_ULONG mask; - if (BN_is_zero(a)) + if (!BN_is_odd(a)) return 0; for (i = a->top - 1; i >= 0; i--) { @@ -1136,12 +1147,13 @@ int BN_GF2m_poly2arr(const BIGNUM *a, int p[], int max) } } - if (k < max) { + if (k > 0 && p[0] > OPENSSL_ECC_MAX_FIELD_BITS) + return 0; + + if (k < max) p[k] = -1; - k++; - } - return k; + return k + 1; } /* diff --git a/test/ec_internal_test.c b/test/ec_internal_test.c index 390f41f9d4..1590a18258 100644 --- a/test/ec_internal_test.c +++ b/test/ec_internal_test.c @@ -150,6 +150,56 @@ static int field_tests_ecp_mont(void) } #ifndef OPENSSL_NO_EC2M +/* Test that decoding of invalid GF2m field parameters fails. */ +static int ec2m_field_sanity(void) +{ + int ret = 0; + BN_CTX *ctx = BN_CTX_new(); + BIGNUM *p, *a, *b; + EC_GROUP *group1 = NULL, *group2 = NULL, *group3 = NULL; + + TEST_info("Testing GF2m hardening\n"); + + BN_CTX_start(ctx); + p = BN_CTX_get(ctx); + a = BN_CTX_get(ctx); + if (!TEST_ptr(b = BN_CTX_get(ctx)) + || !TEST_true(BN_one(a)) + || !TEST_true(BN_one(b))) + goto out; + + /* Even pentanomial value should be rejected */ + if (!TEST_true(BN_set_word(p, 0xf2))) + goto out; + if (!TEST_ptr_null(group1 = EC_GROUP_new_curve_GF2m(p, a, b, ctx))) + TEST_error("Zero constant term accepted in GF2m polynomial"); + + /* Odd hexanomial should also be rejected */ + if (!TEST_true(BN_set_word(p, 0xf3))) + goto out; + if (!TEST_ptr_null(group2 = EC_GROUP_new_curve_GF2m(p, a, b, ctx))) + TEST_error("Hexanomial accepted as GF2m polynomial"); + + /* Excessive polynomial degree should also be rejected */ + if (!TEST_true(BN_set_word(p, 0x71)) + || !TEST_true(BN_set_bit(p, OPENSSL_ECC_MAX_FIELD_BITS + 1))) + goto out; + if (!TEST_ptr_null(group3 = EC_GROUP_new_curve_GF2m(p, a, b, ctx))) + TEST_error("GF2m polynomial degree > %d accepted", + OPENSSL_ECC_MAX_FIELD_BITS); + + ret = group1 == NULL && group2 == NULL && group3 == NULL; + + out: + EC_GROUP_free(group1); + EC_GROUP_free(group2); + EC_GROUP_free(group3); + BN_CTX_end(ctx); + BN_CTX_free(ctx); + + return ret; +} + /* test EC_GF2m_simple_method directly */ static int field_tests_ec2_simple(void) { @@ -367,6 +417,7 @@ int setup_tests(void) ADD_TEST(field_tests_ecp_simple); ADD_TEST(field_tests_ecp_mont); #ifndef OPENSSL_NO_EC2M + ADD_TEST(ec2m_field_sanity); ADD_TEST(field_tests_ec2_simple); #endif ADD_ALL_TESTS(field_tests_default, crv_len); -- Gitee