Rework DSA_size() and ECDSA_size()
authortb <tb@openbsd.org>
Wed, 31 Aug 2022 13:01:01 +0000 (13:01 +0000)
committertb <tb@openbsd.org>
Wed, 31 Aug 2022 13:01:01 +0000 (13:01 +0000)
DSA_size() and ECDSA_size() have a very special hack. They fudge up an
ASN1_INTEGER with a size which is typically > 100 bytes, backed by a
buffer of size 4. This was "fine", however, since they set buf[0] = 0xff,
where the craziness that was i2c_ASN1_INTEGER() only looks at the first
octet (one may then ask why a buffer of size 4 was necessary...).

This changed with the rewrite of i2c_ASN1_INTEGER(), which doesn't
respect this particular hack and rightly assumes that it is fed an
actual ASN1_INTEGER...

Instead, create an appropriate signature and use i2d to determine its
size.

Fixes an out-of-bounds read flagged by ASAN and oss-fuzz.

ok jsing

lib/libcrypto/dsa/dsa_lib.c
lib/libcrypto/ecdsa/ecs_lib.c

index 949722b..da95705 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: dsa_lib.c,v 1.35 2022/06/27 12:28:46 tb Exp $ */
+/* $OpenBSD: dsa_lib.c,v 1.36 2022/08/31 13:01:01 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -220,23 +220,15 @@ DSA_up_ref(DSA *r)
 int
 DSA_size(const DSA *r)
 {
-       int ret, i;
-       ASN1_INTEGER bs;
-       unsigned char buf[4];   /* 4 bytes looks really small.
-                                  However, i2d_ASN1_INTEGER() will not look
-                                  beyond the first byte, as long as the second
-                                  parameter is NULL. */
-
-       i = BN_num_bits(r->q);
-       bs.length = (i + 7) / 8;
-       bs.data = buf;
-       bs.type = V_ASN1_INTEGER;
-       /* If the top bit is set the asn1 encoding is 1 larger. */
-       buf[0] = 0xff;
-
-       i = i2d_ASN1_INTEGER(&bs, NULL);
-       i += i; /* r and s */
-       ret = ASN1_object_size(1, i, V_ASN1_SEQUENCE);
+       DSA_SIG signature;
+       int ret = 0;
+
+       signature.r = r->q;
+       signature.s = r->q;
+
+       if ((ret = i2d_DSA_SIG(&signature, NULL)) < 0)
+               ret = 0;
+
        return ret;
 }
 
index c688a95..18eecba 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: ecs_lib.c,v 1.13 2018/04/14 07:09:21 tb Exp $ */
+/* $OpenBSD: ecs_lib.c,v 1.14 2022/08/31 13:01:01 tb Exp $ */
 /* ====================================================================
  * Copyright (c) 1998-2005 The OpenSSL Project.  All rights reserved.
  *
@@ -197,36 +197,33 @@ ecdsa_check(EC_KEY *key)
 int
 ECDSA_size(const EC_KEY *r)
 {
-       int ret, i;
-       ASN1_INTEGER bs;
-       BIGNUM  *order = NULL;
-       unsigned char buf[4];
+       BIGNUM *order = NULL;
        const EC_GROUP *group;
+       ECDSA_SIG signature;
+       int ret = 0;
 
        if (r == NULL)
-               return 0;
-       group = EC_KEY_get0_group(r);
-       if (group == NULL)
-               return 0;
+               goto err;
+
+       if ((group = EC_KEY_get0_group(r)) == NULL)
+               goto err;
 
        if ((order = BN_new()) == NULL)
-               return 0;
-       if (!EC_GROUP_get_order(group, order, NULL)) {
-               BN_clear_free(order);
-               return 0;
-       }
-       i = BN_num_bits(order);
-       bs.length = (i + 7) / 8;
-       bs.data = buf;
-       bs.type = V_ASN1_INTEGER;
-       /* If the top bit is set the asn1 encoding is 1 larger. */
-       buf[0] = 0xff;
-
-       i = i2d_ASN1_INTEGER(&bs, NULL);
-       i += i; /* r and s */
-       ret = ASN1_object_size(1, i, V_ASN1_SEQUENCE);
+               goto err;
+
+       if (!EC_GROUP_get_order(group, order, NULL))
+               goto err;
+
+       signature.r = order;
+       signature.s = order;
+
+       if ((ret = i2d_ECDSA_SIG(&signature, NULL)) < 0)
+               ret = 0;
+
+ err:
        BN_clear_free(order);
-       return (ret);
+
+       return ret;
 }
 
 int