From: tb <tb@openbsd.org>
Date: Fri, 23 Jun 2023 10:48:40 +0000 (+0000)
Subject: Avoid crash in BN_asc2bn()
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=7b63f6301256b7e76fbac712f5b8acb593267058;p=openbsd

Avoid crash in BN_asc2bn()

Historically (and currently in OpenSSL), BN_asc2bn() could be called with
NULL, but only for positive numbers. So BN_asc2bn(NULL, "1") would succeed
but BN_asc2bn(NULL, "-1"), would crash. The other *2bn functions return a
length, so accepting a NULL makes some sense since it allows callers to
skip over part of the string just parsed (atoi-style).

For BN_asc2bn() a NULL bn makes no sense because it returns a boolean. The
recent CBS rewrite makes BN_asc2bn(NULL, *) always crash which in turn made
Coverity throw a fit.

Another change of behavior from that rewrite pertains to accidents (or is
it madness?) like -0x-11 and 0x-11 being parsed as decimal -17 (which Ingo
of course spotted and diligently documented). This will be addressed later.

ok jsing
---

diff --git a/lib/libcrypto/bn/bn_convert.c b/lib/libcrypto/bn/bn_convert.c
index 9ad9f58f939..4736099cdc6 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.11 2023/06/23 10:33:12 tb Exp $ */
+/* $OpenBSD: bn_convert.c,v 1.12 2023/06/23 10:48:40 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -301,7 +301,8 @@ BN_asc2bn(BIGNUM **bnp, const char *s)
 		return 0;
 
  done:
-	BN_set_negative(*bnp, neg);
+	if (bnp != NULL && *bnp != NULL)
+		BN_set_negative(*bnp, neg);
 
 	return 1;
 }