Rework DSA_size() and ECDSA_size()

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

2 files changed, 32 insertions, 43 deletions

diff --git a/src/lib/libcrypto/dsa/dsa_lib.c b/src/lib/libcrypto/dsa/dsa_lib.c
index 949722b734..da95705947 100644
--- a/src/lib/libcrypto/dsa/dsa_lib.c
+++ b/src/lib/libcrypto/dsa/dsa_lib.c
@@ -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;
 }
 
diff --git a/src/lib/libcrypto/ecdsa/ecs_lib.c b/src/lib/libcrypto/ecdsa/ecs_lib.c
index c688a95f3b..18eecba704 100644
--- a/src/lib/libcrypto/ecdsa/ecs_lib.c
+++ b/src/lib/libcrypto/ecdsa/ecs_lib.c
@@ -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