From 58b76109c097933661021feaa890ac2e80a91357 Mon Sep 17 00:00:00 2001 From: tb Date: Mon, 25 Dec 2023 22:02:59 +0000 Subject: [PATCH] Avoid out-of-bounds accesses in ASN1_BIT_STRING_{get,set}() If a negative n is passed, these functions would underrun the bitstring's data array. So add checks for that and drop spades of unnecessary parens. These functions are quite broken anyway. The setter attempts to zap the unnecessary trailing zero octets, but fails to do so if the bit being cleared isn't already set. Worse is the getter where you can't tell an error (like attempting an out-of-bounds read) from the bit being unset. ok joshua --- lib/libcrypto/asn1/a_bitstr.c | 40 +++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/lib/libcrypto/asn1/a_bitstr.c b/lib/libcrypto/asn1/a_bitstr.c index 7ea3e12b919..851a3a3d514 100644 --- a/lib/libcrypto/asn1/a_bitstr.c +++ b/lib/libcrypto/asn1/a_bitstr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: a_bitstr.c,v 1.41 2023/07/28 10:33:13 tb Exp $ */ +/* $OpenBSD: a_bitstr.c,v 1.42 2023/12/25 22:02:59 tb Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -120,20 +120,24 @@ ASN1_BIT_STRING_set_bit(ASN1_BIT_STRING *a, int n, int value) int w, v, iv; unsigned char *c; - w = n/8; + if (a == NULL) + return 0; + if (n < 0) + return 0; + + w = n / 8; v = 1 << (7 - (n & 0x07)); iv = ~v; - if (!value) - v = 0; - if (a == NULL) - return 0; + if (value == 0) + v = 0; asn1_abs_clear_unused_bits(a); - if ((a->length < (w + 1)) || (a->data == NULL)) { - if (!value) - return(1); /* Don't need to set */ + if (a->length < w + 1 || a->data == NULL) { + /* Don't expand if there's no bit to set. */ + if (value == 0) + return 1; if ((c = recallocarray(a->data, a->length, w + 1, 1)) == NULL) { ASN1error(ERR_R_MALLOC_FAILURE); return 0; @@ -141,11 +145,12 @@ ASN1_BIT_STRING_set_bit(ASN1_BIT_STRING *a, int n, int value) a->data = c; a->length = w + 1; } + a->data[w] = ((a->data[w]) & iv) | v; - while ((a->length > 0) && (a->data[a->length - 1] == 0)) + while (a->length > 0 && a->data[a->length - 1] == 0) a->length--; - return (1); + return 1; } LCRYPTO_ALIAS(ASN1_BIT_STRING_set_bit); @@ -154,11 +159,18 @@ ASN1_BIT_STRING_get_bit(const ASN1_BIT_STRING *a, int n) { int w, v; + if (a == NULL) + return 0; + if (n < 0) + return 0; + w = n / 8; v = 1 << (7 - (n & 0x07)); - if ((a == NULL) || (a->length < (w + 1)) || (a->data == NULL)) - return (0); - return ((a->data[w] & v) != 0); + + if (a->length < w + 1 || a->data == NULL) + return 0; + + return (a->data[w] & v) != 0; } LCRYPTO_ALIAS(ASN1_BIT_STRING_get_bit); -- 2.20.1