Rewrite BN_{asc,dec,hex}2bn() using CBS.

This gives us more readable and safer code. There are two intentional
changes to behaviour - firstly, all three functions zero any BN that was
passed in, prior to doing any further processing. This means that a passed
BN is always in a known state, regardless of what happens later. Secondly,
BN_asc2bn() now fails on NULL input, rather than crashing. This brings its
behaviour inline with BN_dec2bn() and BN_hex2bn().

ok tb@

1 files changed, 224 insertions, 123 deletions

diff --git a/lib/libcrypto/bn/bn_convert.c b/lib/libcrypto/bn/bn_convert.c
index 65834ffd1ed..1a3abbc603d 100644
--- a/lib/libcrypto/bn/bn_convert.c
+++ b/lib/libcrypto/bn/bn_convert.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: bn_convert.c,v 1.8 2023/05/09 05:15:55 jsing Exp $ */
+/* $OpenBSD: bn_convert.c,v 1.9 2023/05/28 10:34:17 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -70,6 +70,9 @@
 #include "bn_local.h"
 #include "bytestring.h"
 
+static int bn_dec2bn_cbs(BIGNUM **bnp, CBS *cbs);
+static int bn_hex2bn_cbs(BIGNUM **bnp, CBS *cbs);
+
 static const char hex_digits[] = "0123456789ABCDEF";
 
 typedef enum {
@@ -253,21 +256,53 @@ BN_lebin2bn(const unsigned char *s, int len, BIGNUM *ret)
 }
 
 int
-BN_asc2bn(BIGNUM **bn, const char *a)
+BN_asc2bn(BIGNUM **bnp, const char *s)
 {
-	const char *p = a;
-	if (*p == '-')
-		p++;
+	CBS cbs, cbs_hex;
+	size_t s_len;
+	uint8_t v;
+	int neg;
 
-	if (p[0] == '0' && (p[1] == 'X' || p[1] == 'x')) {
-		if (!BN_hex2bn(bn, p + 2))
-			return 0;
-	} else {
-		if (!BN_dec2bn(bn, p))
+	if (bnp != NULL && *bnp != NULL)
+		BN_zero(*bnp);
+
+	if (s == NULL)
+		return 0;
+	if ((s_len = strlen(s)) == 0)
+		return 0;
+
+	CBS_init(&cbs, s, s_len);
+
+	/* Handle negative sign. */
+	if (!CBS_peek_u8(&cbs, &v))
+		return 0;
+	if ((neg = (v == '-'))) {
+		if (!CBS_skip(&cbs, 1))
 			return 0;
 	}
-	if (*a == '-')
-		BN_set_negative(*bn, 1);
+
+	/* Try parsing as hexidecimal with a 0x prefix. */
+	CBS_dup(&cbs, &cbs_hex);
+	if (!CBS_get_u8(&cbs_hex, &v))
+		goto decimal;
+	if (v != '0')
+		goto decimal;
+	if (!CBS_get_u8(&cbs_hex, &v))
+		goto decimal;
+	if (v != 'X' && v != 'x')
+		goto decimal;
+	if (!bn_hex2bn_cbs(bnp, &cbs_hex))
+		return 0;
+
+	goto done;
+
+ decimal:
+	if (!bn_dec2bn_cbs(bnp, &cbs))
+		return 0;
+
+ done:
+	BN_set_negative(*bnp, neg);
+
 	return 1;
 }
 
@@ -349,73 +384,108 @@ BN_bn2dec(const BIGNUM *bn)
 	return s;
 }
 
-int
-BN_dec2bn(BIGNUM **bn, const char *a)
+static int
+bn_dec2bn_cbs(BIGNUM **bnp, CBS *cbs)
 {
-	BIGNUM *ret = NULL;
-	BN_ULONG l = 0;
-	int neg = 0, i, j;
-	int num;
+	CBS cbs_digits;
+	BIGNUM *bn = NULL;
+	int d, neg, num;
+	size_t digits = 0;
+	BN_ULONG w;
+	uint8_t v;
 
-	if ((a == NULL) || (*a == '\0'))
-		return (0);
-	if (*a == '-') {
-		neg = 1;
-		a++;
+	/* Handle negative sign. */
+	if (!CBS_peek_u8(cbs, &v))
+		goto err;
+	if ((neg = (v == '-'))) {
+		if (!CBS_skip(cbs, 1))
+			goto err;
 	}
 
-	for (i = 0; i <= (INT_MAX / 4) && isdigit((unsigned char)a[i]); i++)
-		;
-	if (i > INT_MAX / 4)
-		return (0);
-
-	num = i + neg;
-	if (bn == NULL)
-		return (num);
-
-	/* a is the start of the digits, and it is 'i' long.
-	 * We chop it into BN_DEC_NUM digits at a time */
-	if (*bn == NULL) {
-		if ((ret = BN_new()) == NULL)
-			return (0);
-	} else {
-		ret = *bn;
-		BN_zero(ret);
+	/* Scan to find last decimal digit. */
+	CBS_dup(cbs, &cbs_digits);
+	while (CBS_len(&cbs_digits) > 0) {
+		if (!CBS_get_u8(&cbs_digits, &v))
+			goto err;
+		if (!isdigit(v))
+			break;
+		digits++;
 	}
+	if (digits > INT_MAX / 4)
+		goto err;
 
-	/* i is the number of digits, a bit of an over expand */
-	if (!bn_expand(ret, i * 4))
+	num = digits + neg;
+
+	if (bnp == NULL)
+		return num;
+
+	if ((bn = *bnp) == NULL)
+		bn = BN_new();
+	if (bn == NULL)
+		goto err;
+	if (!bn_expand(bn, digits * 4))
 		goto err;
 
-	j = BN_DEC_NUM - (i % BN_DEC_NUM);
-	if (j == BN_DEC_NUM)
-		j = 0;
-	l = 0;
-	while (*a) {
-		l *= 10;
-		l += *a - '0';
-		a++;
-		if (++j == BN_DEC_NUM) {
-			if (!BN_mul_word(ret, BN_DEC_CONV))
+	if ((d = digits % BN_DEC_NUM) == 0)
+		d = BN_DEC_NUM;
+
+	w = 0;
+
+	/* Work forwards from most significant digit. */
+	while (digits-- > 0) {
+		if (!CBS_get_u8(cbs, &v))
+			goto err;
+
+		if (v < '0' || v > '9')
+			goto err;
+
+		v -= '0';
+		w = w * 10 + v;
+		d--;
+
+		if (d == 0) {
+			if (!BN_mul_word(bn, BN_DEC_CONV))
 				goto err;
-			if (!BN_add_word(ret, l))
+			if (!BN_add_word(bn, w))
 				goto err;
-			l = 0;
-			j = 0;
+
+			d = BN_DEC_NUM;
+			w = 0;
 		}
 	}
 
-	bn_correct_top(ret);
+	bn_correct_top(bn);
 
-	BN_set_negative(ret, neg);
+	BN_set_negative(bn, neg);
 
-	*bn = ret;
-	return (num);
+	*bnp = bn;
 
-err:
-	if (*bn == NULL)
-		BN_free(ret);
-	return (0);
+	return num;
+
+ err:
+	if (bnp != NULL && *bnp == NULL)
+		BN_free(bn);
+
+	return 0;
+}
+
+int
+BN_dec2bn(BIGNUM **bnp, const char *s)
+{
+	size_t s_len;
+	CBS cbs;
+
+	if (bnp != NULL && *bnp != NULL)
+		BN_zero(*bnp);
+
+	if (s == NULL)
+		return 0;
+	if ((s_len = strlen(s)) == 0)
+		return 0;
+
+	CBS_init(&cbs, s, s_len);
+
+	return bn_dec2bn_cbs(bnp, &cbs);
 }
 
 char *
@@ -463,81 +533,112 @@ BN_bn2hex(const BIGNUM *bn)
 	return s;
 }
 
-int
-BN_hex2bn(BIGNUM **bn, const char *a)
+static int
+bn_hex2bn_cbs(BIGNUM **bnp, CBS *cbs)
 {
-	BIGNUM *ret = NULL;
-	BN_ULONG l = 0;
-	int neg = 0, h, m, i,j, k, c;
-	int num;
+	CBS cbs_digits;
+	BIGNUM *bn = NULL;
+	int b, i, neg, num;
+	size_t digits = 0;
+	BN_ULONG w;
+	uint8_t v;
 
-	if ((a == NULL) || (*a == '\0'))
-		return (0);
+	/* Handle negative sign. */
+	if (!CBS_peek_u8(cbs, &v))
+		goto err;
+	if ((neg = (v == '-'))) {
+		if (!CBS_skip(cbs, 1))
+			goto err;
+	}
 
-	if (*a == '-') {
-		neg = 1;
-		a++;
+	/* Scan to find last hexadecimal digit. */
+	CBS_dup(cbs, &cbs_digits);
+	while (CBS_len(&cbs_digits) > 0) {
+		if (!CBS_get_u8(&cbs_digits, &v))
+			goto err;
+		if (!isxdigit(v))
+			break;
+		digits++;
 	}
+	if (digits > INT_MAX / 4)
+		goto err;
 
-	for (i = 0; i <= (INT_MAX / 4) && isxdigit((unsigned char)a[i]); i++)
-		;
-	if (i > INT_MAX / 4)
-		return (0);
+	num = digits + neg;
+
+	if (bnp == NULL)
+		return num;
 
-	num = i + neg;
+	if ((bn = *bnp) == NULL)
+		bn = BN_new();
 	if (bn == NULL)
-		return (num);
-
-	/* a is the start of the hex digits, and it is 'i' long */
-	if (*bn == NULL) {
-		if ((ret = BN_new()) == NULL)
-			return (0);
-	} else {
-		ret = *bn;
-		BN_zero(ret);
-	}
+		goto err;
+	if (!bn_expand(bn, digits * 4))
+		goto err;
 
-	/* i is the number of hex digits */
-	if (!bn_expand(ret, i * 4))
+	if (!CBS_get_bytes(cbs, cbs, digits))
 		goto err;
 
-	j = i; /* least significant 'hex' */
-	m = 0;
-	h = 0;
-	while (j > 0) {
-		m = ((BN_BYTES * 2) <= j) ? (BN_BYTES * 2) : j;
-		l = 0;
-		for (;;) {
-			c = a[j - m];
-			if ((c >= '0') && (c <= '9'))
-				k = c - '0';
-			else if ((c >= 'a') && (c <= 'f'))
-				k = c - 'a' + 10;
-			else if ((c >= 'A') && (c <= 'F'))
-				k = c - 'A' + 10;
-			else
-				k = 0; /* paranoia */
-			l = (l << 4) | k;
-
-			if (--m <= 0) {
-				ret->d[h++] = l;
-				break;
-			}
+	b = BN_BITS2;
+	i = 0;
+	w = 0;
+
+	/* Work backwards from least significant digit. */
+	while (digits-- > 0) {
+		if (!CBS_get_last_u8(cbs, &v))
+			goto err;
+
+		if (v >= '0' && v <= '9')
+			v -= '0';
+		else if (v >= 'a' && v <= 'f')
+			v -= 'a' - 10;
+		else if (v >= 'A' && v <= 'F')
+			v -= 'A' - 10;
+		else
+			goto err;
+
+		w |= (BN_ULONG)v << (BN_BITS2 - b);
+		b -= 4;
+
+		if (b == 0 || digits == 0) {
+			b = BN_BITS2;
+			bn->d[i++] = w;
+			w = 0;
 		}
-		j -= (BN_BYTES * 2);
 	}
-	ret->top = h;
-	bn_correct_top(ret);
 
-	BN_set_negative(ret, neg);
+	bn->top = i;
+	bn_correct_top(bn);
+
+	BN_set_negative(bn, neg);
+
+	*bnp = bn;
+
+	return num;
+
+ err:
+	if (bnp != NULL && *bnp == NULL)
+		BN_free(bn);
+
+	return 0;
+}
+
+int
+BN_hex2bn(BIGNUM **bnp, const char *s)
+{
+	size_t s_len;
+	CBS cbs;
+
+	if (bnp != NULL && *bnp != NULL)
+		BN_zero(*bnp);
+
+	if (s == NULL)
+		return 0;
+	if ((s_len = strlen(s)) == 0)
+		return 0;
 
-	*bn = ret;
-	return (num);
+	CBS_init(&cbs, s, s_len);
 
-err:
-	if (*bn == NULL)
-		BN_free(ret);
-	return (0);
+	return bn_hex2bn_cbs(bnp, &cbs);
 }
 
 int