From: tb <tb@openbsd.org>
Date: Wed, 31 Aug 2022 13:01:01 +0000 (+0000)
Subject: Rework DSA_size() and ECDSA_size()
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=135ec00f7d18dc23768b653aa1275b39b2d030c5;p=openbsd

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
---

diff --git a/lib/libcrypto/dsa/dsa_lib.c b/lib/libcrypto/dsa/dsa_lib.c
index 949722b7345..da95705947f 100644
--- a/lib/libcrypto/dsa/dsa_lib.c
+++ b/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/lib/libcrypto/ecdsa/ecs_lib.c b/lib/libcrypto/ecdsa/ecs_lib.c
index c688a95f3b2..18eecba704b 100644
--- a/lib/libcrypto/ecdsa/ecs_lib.c
+++ b/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