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 9ad9f58..4736099 100644 (file)
--- 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;
 }