Align hex_to_string with OpenSSL 1.1 behavior
authortb <tb@openbsd.org>
Sun, 23 Apr 2023 10:19:52 +0000 (10:19 +0000)
committertb <tb@openbsd.org>
Sun, 23 Apr 2023 10:19:52 +0000 (10:19 +0000)
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

index 1a043d9..b6f1919 100644 (file)
@@ -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;
        }