From 7b63f6301256b7e76fbac712f5b8acb593267058 Mon Sep 17 00:00:00 2001 From: tb Date: Fri, 23 Jun 2023 10:48:40 +0000 Subject: [PATCH] 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 --- lib/libcrypto/bn/bn_convert.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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; } -- 2.20.1