From 6538316403f5b4a503c4cfbad96069b4942cf92e Mon Sep 17 00:00:00 2001 From: tb Date: Fri, 12 May 2023 13:56:17 +0000 Subject: [PATCH] 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 --- lib/libcrypto/x509/x509_utl.c | 194 ++++++++++++++++++++++------------ 1 file changed, 124 insertions(+), 70 deletions(-) diff --git a/lib/libcrypto/x509/x509_utl.c b/lib/libcrypto/x509/x509_utl.c index 0cc5fb40685..28e9179e95b 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.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 +#include #include #include @@ -67,6 +67,8 @@ #include #include +#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); -- 2.20.1