From 5517f1c85f08bba410000e2bfa9a87829f1a34db Mon Sep 17 00:00:00 2001 From: tb Date: Sun, 23 Apr 2023 10:19:52 +0000 Subject: [PATCH] Align hex_to_string with OpenSSL 1.1 behavior This is a bit of a strange one. When this function was moved and renamed, somehow checks for NULL pointers and 0 length were lost. Whether that was due to great review missing it or great review suggesting it, is unclear. Now the function can actually legitimately be called with a length of 0 (as ASN.1 OCTET STRINGS can have length 0) and "" is the appropriate representation for that, so the fix is to allocate a 0 octet. That much was correct. What was completely missed is that a long can be negative which will then still lead to an out-of-bounds access. So fix that as well. Finally, don't malloc 3 * len + 1 without overflow checking. Rather use calloc's internal checks. The + 1 isn't really needed anyway. All this is still really gross and can be done much more cleanly and safely with CBB/CBS. This will done later once we have better regress coverage. ok jsing --- lib/libcrypto/x509/x509_utl.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/libcrypto/x509/x509_utl.c b/lib/libcrypto/x509/x509_utl.c index 1a043d93d57..b6f1919b39f 100644 --- a/lib/libcrypto/x509/x509_utl.c +++ b/lib/libcrypto/x509/x509_utl.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_utl.c,v 1.9 2023/04/23 09:58:38 tb Exp $ */ +/* $OpenBSD: x509_utl.c,v 1.10 2023/04/23 10:19:52 tb Exp $ */ /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL * project. */ @@ -471,9 +471,11 @@ hex_to_string(const unsigned char *buffer, long len) int i; static const char hexdig[] = "0123456789ABCDEF"; - if (!buffer || !len) + if (len < 0) return NULL; - if (!(tmp = malloc(len * 3 + 1))) { + if (len == 0) + return calloc(1, 1); + if ((tmp = calloc(len, 3)) == NULL) { X509V3error(ERR_R_MALLOC_FAILURE); return NULL; } -- 2.20.1