Avoid infinite loop for custom curves of order 1
authortb <tb@openbsd.org>
Thu, 7 Apr 2022 17:37:25 +0000 (17:37 +0000)
committertb <tb@openbsd.org>
Thu, 7 Apr 2022 17:37:25 +0000 (17:37 +0000)
If a private key encoded with EC parameters happens to have
order 1 and is used for ECDSA signatures, this causes an
infinite loop since a random integer x in the interval [0,1)
will be 0, so do ... while (x == 0); will loop indefinitely.

Found and reported with a reproducer by Hanno Boeck.
Helpful comments and analysis from David Benjamin.

ok beck jsing

lib/libcrypto/ec/ec_lib.c
lib/libcrypto/ecdsa/ecs_ossl.c

index 888f1ed..4ec17d5 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: ec_lib.c,v 1.44 2022/03/29 14:03:12 tb Exp $ */
+/* $OpenBSD: ec_lib.c,v 1.45 2022/04/07 17:37:25 tb Exp $ */
 /*
  * Originally written by Bodo Moeller for the OpenSSL project.
  */
@@ -348,10 +348,10 @@ EC_GROUP_set_generator(EC_GROUP *group, const EC_POINT *generator,
        }
 
        /*
-        * Require order >= 1 and enforce an upper bound of at most one bit more
+        * Require order > 1 and enforce an upper bound of at most one bit more
         * than the field cardinality due to Hasse's theorem.
         */
-       if (order == NULL || BN_is_zero(order) || BN_is_negative(order) ||
+       if (order == NULL || BN_cmp(order, BN_value_one()) <= 0 ||
            BN_num_bits(order) > BN_num_bits(&group->field) + 1) {
                ECerror(EC_R_INVALID_GROUP_ORDER);
                return 0;
index 2429e36..0203b01 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: ecs_ossl.c,v 1.23 2022/01/20 11:03:48 inoguchi Exp $ */
+/* $OpenBSD: ecs_ossl.c,v 1.24 2022/04/07 17:37:25 tb Exp $ */
 /*
  * Written by Nils Larsch for the OpenSSL project
  */
@@ -163,6 +163,11 @@ ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in, BIGNUM **kinvp, BIGNUM **rp)
                goto err;
        }
 
+       if (BN_cmp(order, BN_value_one()) <= 0) {
+               ECDSAerror(EC_R_INVALID_GROUP_ORDER);
+               goto err;
+       }
+
        /* Preallocate space. */
        order_bits = BN_num_bits(order);
        if (!BN_set_bit(k, order_bits) ||