From 135ec00f7d18dc23768b653aa1275b39b2d030c5 Mon Sep 17 00:00:00 2001 From: tb Date: Wed, 31 Aug 2022 13:01:01 +0000 Subject: [PATCH] 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 --- lib/libcrypto/dsa/dsa_lib.c | 28 ++++++++------------- lib/libcrypto/ecdsa/ecs_lib.c | 47 ++++++++++++++++------------------- 2 files changed, 32 insertions(+), 43 deletions(-) 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 -- 2.20.1