Fix x509v3_bytes_to_hex when passed the empty string.

Change-Id: I0a27b98ea1b8fd28ec415cb6c9ca789d438dd545
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51627
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

crypto/x509/x509_test.cc

@@ -3891,3 +3891,23 @@ TEST(X509Test, AddDuplicates) {
 
   EXPECT_EQ(sk_X509_OBJECT_num(X509_STORE_get0_objects(store.get())), 2u);
 }
+
+TEST(X509Test, BytesToHex) {
+  struct {
+    std::vector<uint8_t> bytes;
+    const char *hex;
+  } kTests[] = {
+      {{}, ""},
+      {{0x00}, "00"},
+      {{0x00, 0x11, 0x22}, "00:11:22"},
+      {{0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef},
+       "01:23:45:67:89:AB:CD:EF"},
+  };
+  for (const auto &t : kTests) {
+    SCOPED_TRACE(Bytes(t.bytes));
+    bssl::UniquePtr<char> hex(
+        x509v3_bytes_to_hex(t.bytes.data(), t.bytes.size()));
+    ASSERT_TRUE(hex);
+    EXPECT_STREQ(hex.get(), t.hex);
+  }
+}

crypto/x509v3/internal.h

@@ -70,21 +70,21 @@ extern "C" {
 #endif
 
 
-// x509v3_bytes_to_hex encodes |len| bytes from |buffer| to hex and returns a
+// x509v3_bytes_to_hex encodes |len| bytes from |in| to hex and returns a
 // newly-allocated NUL-terminated string containing the result, or NULL on
 // allocation error.
 //
-// Note this function was historically named |hex_to_string| in OpenSSL, not
+// This function was historically named |hex_to_string| in OpenSSL. Despite the
-// |string_to_hex|.
+// name, |hex_to_string| converted to hex.
-char *x509v3_bytes_to_hex(const unsigned char *buffer, long len);
+OPENSSL_EXPORT char *x509v3_bytes_to_hex(const uint8_t *in, size_t len);
 
 // x509v3_hex_string_to_bytes decodes |str| in hex and returns a newly-allocated
 // array containing the result, or NULL on error. On success, it sets |*len| to
 // the length of the result. Colon separators between bytes in the input are
 // allowed and ignored.
 //
-// Note this function was historically named |string_to_hex| in OpenSSL, not
+// This function was historically named |string_to_hex| in OpenSSL. Despite the
-// |hex_to_string|.
+// name, |string_to_hex| converted from hex.
 unsigned char *x509v3_hex_to_bytes(const char *str, long *len);
 
 // x509v3_name_cmp returns zero if |name| is equal to |cmp| or begins with |cmp|

crypto/x509v3/v3_utl.c

@@ -63,6 +63,7 @@
 #include <string.h>
 
 #include <openssl/bn.h>
+#include <openssl/bytestring.h>
 #include <openssl/conf.h>
 #include <openssl/err.h>
 #include <openssl/mem.h>
@@ -467,33 +468,33 @@ static char *strip_spaces(char *name)
 
 /* hex string utilities */
 
-/*
+char *x509v3_bytes_to_hex(const uint8_t *in, size_t len)
- * Given a buffer of length 'len' return a OPENSSL_malloc'ed string with its
- * hex representation @@@ (Contents of buffer are always kept in ASCII, also
- * on EBCDIC machines)
- */
-
-char *x509v3_bytes_to_hex(const unsigned char *buffer, long len)
 {
-    char *tmp, *q;
+    CBB cbb;
-    const unsigned char *p;
+    if (!CBB_init(&cbb, len * 3 + 1)) {
-    int i;
+        goto err;
-    static const char hexdig[] = "0123456789ABCDEF";
-    if (!buffer || !len)
-        return NULL;
-    if (!(tmp = OPENSSL_malloc(len * 3 + 1))) {
-        OPENSSL_PUT_ERROR(X509V3, ERR_R_MALLOC_FAILURE);
-        return NULL;
     }
-    q = tmp;
+    for (size_t i = 0; i < len; i++) {
-    for (i = 0, p = buffer; i < len; i++, p++) {
+        static const char hex[] = "0123456789ABCDEF";
-        *q++ = hexdig[(*p >> 4) & 0xf];
+        if ((i > 0 && !CBB_add_u8(&cbb, ':')) ||
-        *q++ = hexdig[*p & 0xf];
+            !CBB_add_u8(&cbb, hex[in[i] >> 4]) ||
-        *q++ = ':';
+            !CBB_add_u8(&cbb, hex[in[i] & 0xf])) {
+            goto err;
+        }
+    }
+    uint8_t *ret;
+    size_t unused_len;
+    if (!CBB_add_u8(&cbb, 0) ||
+        !CBB_finish(&cbb, &ret, &unused_len)) {
+        goto err;
     }
-    q[-1] = 0;
 
-    return tmp;
+    return (char *)ret;
+
+err:
+    OPENSSL_PUT_ERROR(X509V3, ERR_R_MALLOC_FAILURE);
+    CBB_cleanup(&cbb);
+    return NULL;
 }
 
 unsigned char *x509v3_hex_to_bytes(const char *str, long *len)