Rewrite string_to_hex() and hex_to_string() using CBB/CBS

These helpers used to contain messy pointer bashing some with weird logic
for NUL termination. This can be written more safely and cleanly using
CBB/CBS, so do that. The result is nearly but not entirely identical to
code used elsewhere due to some strange semantics. Apart from errors pushed
on the stack due to out-of-memory conditions, care was taken to preserve
error codes.

ok jsing

diff --git a/lib/libcrypto/x509/x509_utl.c b/lib/libcrypto/x509/x509_utl.c
index 0cc5fb4..28e9179 100644 (file)
--- a/lib/libcrypto/x509/x509_utl.c
+++ b/lib/libcrypto/x509/x509_utl.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_utl.c,v 1.14 2023/04/23 11:52:14 tb Exp $ */
+/* $OpenBSD: x509_utl.c,v 1.15 2023/05/12 13:56:17 tb Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project.
  */
@@ -55,9 +55,9 @@
  * Hudson (tjh@cryptsoft.com).
  *
  */
-/* X509 v3 extension utilities */
 
 #include <ctype.h>
+#include <limits.h>
 #include <stdio.h>
 #include <string.h>
 
@@ -67,6 +67,8 @@
 #include <openssl/err.h>
 #include <openssl/x509v3.h>
 
+#include "bytestring.h"
+
 static char *bn_to_string(const BIGNUM *bn);
 static char *strip_spaces(char *name);
 static int sk_strcmp(const char * const *a, const char * const *b);
@@ -459,97 +461,149 @@ strip_spaces(char *name)
        return p;
 }
 
-/* hex string utilities */
+static const char hex_digits[] = "0123456789ABCDEF";
 
-/* Given a buffer of length 'len' return a malloc'ed string with its
- * hex representation
- */
 char *
 hex_to_string(const unsigned char *buffer, long len)
 {
-       char *tmp, *q;
-       const unsigned char *p;
-       int i;
-       static const char hexdig[] = "0123456789ABCDEF";
+       CBB cbb;
+       CBS cbs;
+       uint8_t *out = NULL;
+       uint8_t c;
+       size_t out_len;
+
+       if (!CBB_init(&cbb, 0))
+               goto err;
 
        if (len < 0)
-               return NULL;
-       if (len == 0)
-               return calloc(1, 1);
-       if ((tmp = calloc(len, 3)) == NULL) {
-               X509V3error(ERR_R_MALLOC_FAILURE);
-               return NULL;
-       }
-       q = tmp;
-       for (i = 0, p = buffer; i < len; i++, p++) {
-               *q++ = hexdig[(*p >> 4) & 0xf];
-               *q++ = hexdig[*p & 0xf];
-               *q++ = ':';
+               goto err;
+
+       CBS_init(&cbs, buffer, len);
+       while (CBS_len(&cbs) > 0) {
+               if (!CBS_get_u8(&cbs, &c))
+                       goto err;
+               if (!CBB_add_u8(&cbb, hex_digits[c >> 4]))
+                       goto err;
+               if (!CBB_add_u8(&cbb, hex_digits[c & 0xf]))
+                       goto err;
+               if (CBS_len(&cbs) > 0) {
+                       if (!CBB_add_u8(&cbb, ':'))
+                               goto err;
+               }
        }
-       q[-1] = 0;
-       return tmp;
+
+       if (!CBB_add_u8(&cbb, '\0'))
+               goto err;
+
+       if (!CBB_finish(&cbb, &out, &out_len))
+               goto err;
+
+ err:
+       CBB_cleanup(&cbb);
+
+       return out;
 }
 LCRYPTO_ALIAS(hex_to_string);
 
-/* Give a string of hex digits convert to
- * a buffer
- */
+static int
+x509_skip_colons_cbs(CBS *cbs)
+{
+       uint8_t c;
+
+       while (CBS_len(cbs) > 0) {
+               if (!CBS_peek_u8(cbs, &c))
+                       return 0;
+               if (c != ':')
+                       return 1;
+               if (!CBS_get_u8(cbs, &c))
+                       return 0;
+       }
+
+       return 1;
+
+}
+
+static int
+x509_get_xdigit_nibble_cbs(CBS *cbs, uint8_t *out_nibble) {
+       uint8_t c;
+
+       if (!CBS_get_u8(cbs, &c))
+               return 0;
+
+       if (c >= '0' && c <= '9') {
+               *out_nibble = c - '0';
+               return 1;
+       }
+       if (c >= 'a' && c <= 'f') {
+               *out_nibble = c - 'a' + 10;
+               return 1;
+       }
+       if (c >= 'A' && c <= 'F') {
+               *out_nibble = c - 'A' + 10;
+               return 1;
+       }
+
+       X509V3error(X509V3_R_ILLEGAL_HEX_DIGIT);
+       return 0;
+
+}
 
 unsigned char *
 string_to_hex(const char *str, long *len)
 {
-       unsigned char *hexbuf, *q;
-       unsigned char ch, cl, *p;
-       if (!str) {
-               X509V3error(X509V3_R_INVALID_NULL_ARGUMENT);
-               return NULL;
-       }
-       if (!(hexbuf = malloc(strlen(str) >> 1)))
-               goto err;
-       for (p = (unsigned char *)str, q = hexbuf; *p; ) {
-               ch = *p++;
-               if (ch == ':')
-                       continue;
-               cl = *p++;
-               if (!cl) {
-                       X509V3error(X509V3_R_ODD_NUMBER_OF_DIGITS);
-                       free(hexbuf);
-                       return NULL;
-               }
-               ch = tolower(ch);
-               cl = tolower(cl);
+       CBB cbb;
+       CBS cbs;
+       uint8_t *out = NULL;
+       size_t out_len;
+       uint8_t hi, lo;
 
-               if ((ch >= '0') && (ch <= '9'))
-                       ch -= '0';
-               else if ((ch >= 'a') && (ch <= 'f'))
-                       ch -= 'a' - 10;
-               else
-                       goto badhex;
+       *len = 0;
 
-               if ((cl >= '0') && (cl <= '9'))
-                       cl -= '0';
-               else if ((cl >= 'a') && (cl <= 'f'))
-                       cl -= 'a' - 10;
-               else
-                       goto badhex;
+       if (!CBB_init(&cbb, 0))
+               goto err;
 
-               *q++ = (ch << 4) | cl;
+       if (str == NULL) {
+               X509V3error(X509V3_R_INVALID_NULL_ARGUMENT);
+               goto err;
+       }
+
+       CBS_init(&cbs, str, strlen(str));
+       while (CBS_len(&cbs) > 0) {
+               /*
+                * Skipping only a single colon between two pairs of digits
+                * would make more sense - history...
+                */
+               if (!x509_skip_colons_cbs(&cbs))
+                       goto err;
+               /* Another historic idiocy. */
+               if (CBS_len(&cbs) == 0)
+                       break;
+               if (!x509_get_xdigit_nibble_cbs(&cbs, &hi))
+                       goto err;
+               if (CBS_len(&cbs) == 0) {
+                       X509error(X509V3_R_ODD_NUMBER_OF_DIGITS);
+                       goto err;
+               }
+               if (!x509_get_xdigit_nibble_cbs(&cbs, &lo))
+                       goto err;
+               if (!CBB_add_u8(&cbb, hi << 4 | lo))
+                       goto err;
        }
 
-       if (len)
-               *len = q - hexbuf;
+       if (!CBB_finish(&cbb, &out, &out_len))
+               goto err;
+       if (out_len > LONG_MAX) {
+               freezero(out, out_len);
+               out = NULL;
+               goto err;
+       }
 
-       return hexbuf;
+       *len = out_len;
 
  err:
-       free(hexbuf);
-       X509V3error(ERR_R_MALLOC_FAILURE);
-       return NULL;
+       CBB_cleanup(&cbb);
 
- badhex:
-       free(hexbuf);
-       X509V3error(X509V3_R_ILLEGAL_HEX_DIGIT);
-       return NULL;
+       return out;
 }
 LCRYPTO_ALIAS(string_to_hex);