From b7ec529f61b958300f40054708715ee243aeb00d Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Wed, 29 Jun 2022 18:01:27 -0500 Subject: [PATCH 1/6] wolfcrypt/src/siphash.c: in wc_SipHash(), use FALL_THROUGH macro, not /* fall-through */. --- wolfcrypt/src/siphash.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/wolfcrypt/src/siphash.c b/wolfcrypt/src/siphash.c index 021a3115b..ef1f6179a 100644 --- a/wolfcrypt/src/siphash.c +++ b/wolfcrypt/src/siphash.c @@ -886,19 +886,19 @@ int wc_SipHash(const unsigned char* key, const unsigned char* in, word32 inSz, switch (inSz) { case 7: b |= (word64)in[6] << 48; - /* fall-through */ + FALL_THROUGH; case 6: b |= (word64)in[5] << 40; - /* fall-through */ + FALL_THROUGH; case 5: b |= (word64)in[4] << 32; - /* fall-through */ + FALL_THROUGH; case 4: b |= (word64)GET_U32(in); break; case 3: b |= (word64)in[2] << 16; - /* fall-through */ + FALL_THROUGH; case 2: b |= (word64)GET_U16(in); break; From 5bd8288b370a00440e11932781abe28dd47b65db Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Wed, 29 Jun 2022 18:03:24 -0500 Subject: [PATCH 2/6] fix printed-null bug in wolfssl_print_number(). --- src/pk.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pk.c b/src/pk.c index 2bcb9e809..d7702dd33 100644 --- a/src/pk.c +++ b/src/pk.c @@ -267,8 +267,8 @@ static int wolfssl_print_number(WOLFSSL_BIO* bio, mp_int* num, const char* name, for (i = 0; (ret == 1) && (i < rawLen); i++) { /* Check for a complete line. */ if (li >= PRINT_NUM_MAX_DIGIT_LINE) { - /* More bytes coming so append carriage return. */ - line[li++] = '\n'; + /* More bytes coming so change the trailing ':' to a line break. */ + line[li-1] = '\n'; /* Write out the line. */ if (wolfSSL_BIO_write(bio, line, li) <= 0) { ret = 0; From 28213ad19813b443ac13372035a777f3f035ec40 Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Wed, 29 Jun 2022 18:11:28 -0500 Subject: [PATCH 3/6] src/x509.c: fix wolfSSL_X509_signature_print() to print raw signature algorithm as hex digits, not as an (unprintable) string; fix printed-null bug in wolfSSL_X509_NAME_print_ex() (relates particularly to calls from wolfSSL_X509_NAME_print_ex_fp()). --- src/x509.c | 39 ++++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/src/x509.c b/src/x509.c index 1e6ff1e2c..27efc1280 100644 --- a/src/x509.c +++ b/src/x509.c @@ -6231,6 +6231,10 @@ int wolfSSL_X509_print_fp(XFILE fp, WOLFSSL_X509 *x509) int wolfSSL_X509_signature_print(WOLFSSL_BIO *bp, const WOLFSSL_X509_ALGOR *sigalg, const WOLFSSL_ASN1_STRING *sig) { + int length = 0; + word32 idx = 0; + int i; + (void)sig; WOLFSSL_ENTER("wolfSSL_X509_signature_print"); @@ -6240,16 +6244,37 @@ int wolfSSL_X509_signature_print(WOLFSSL_BIO *bp, return WOLFSSL_FAILURE; } - if (wolfSSL_BIO_puts(bp, " Signature Algorithm: ") <= 0) { + if ((sigalg->algorithm->obj == NULL) || (sigalg->algorithm->obj[idx++] != ASN_OBJECT_ID)) { + WOLFSSL_MSG("Bad ASN1 Object"); + return WOLFSSL_FAILURE; + } + + if (GetLength((const byte*)sigalg->algorithm->obj, &idx, &length, + sigalg->algorithm->objSz) < 0 || length < 0) { + return WOLFSSL_FAILURE; + } + + if (wolfSSL_BIO_puts(bp, " Raw Signature Algorithm:") <= 0) { WOLFSSL_MSG("wolfSSL_BIO_puts error"); return WOLFSSL_FAILURE; } - if (wolfSSL_i2a_ASN1_OBJECT(bp, sigalg->algorithm) <= 0) { - WOLFSSL_MSG("wolfSSL_i2a_ASN1_OBJECT error"); - return WOLFSSL_FAILURE; + for (i = 0; i < length; ++i) { + char hex_digits[4]; +#ifdef XSNPRINTF + XSNPRINTF(hex_digits, sizeof hex_digits, "%c%02X", i>0 ? ':' : ' ', + (unsigned int)sigalg->algorithm->obj[idx+i]); +#else + XSPRINTF(hex_digits, "%c%02X", i>0 ? ':' : ' ', + (unsigned int)sigalg->algorithm->obj[idx+i]); +#endif + if (wolfSSL_BIO_puts(bp, hex_digits) <= 0) + return WOLFSSL_FAILURE; } + if (wolfSSL_BIO_puts(bp, "\n") <= 0) + return WOLFSSL_FAILURE; + return WOLFSSL_SUCCESS; } #endif /* !NO_BIO */ @@ -11386,7 +11411,11 @@ int wolfSSL_X509_NAME_print_ex(WOLFSSL_BIO* bio, WOLFSSL_X509_NAME* name, } else { XSNPRINTF(tmp, tmpSz, "%s=%s", buf, nameStr); - tmpSz = len + nameStrSz + 2; /* 2 for '=', '\0' */ + tmpSz = len + nameStrSz + 1; /* 1 for '=' */ + if (bio->type != WOLFSSL_BIO_FILE) + ++tmpSz; /* include the terminating null when not writing to a + * file. + */ } if (wolfSSL_BIO_write(bio, tmp, tmpSz) != tmpSz) { From 1a9388b935bb3db8b33ed5b7de2b6cc424ecc631 Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Wed, 29 Jun 2022 22:45:30 -0500 Subject: [PATCH 4/6] src/pk.c: fix misuses around snprintf(). --- src/pk.c | 66 ++++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 48 insertions(+), 18 deletions(-) diff --git a/src/pk.c b/src/pk.c index d7702dd33..903b652a7 100644 --- a/src/pk.c +++ b/src/pk.c @@ -151,10 +151,15 @@ static int wolfssl_print_indent(WOLFSSL_BIO* bio, char* line, int lineLen, if (indent > 0) { /* Print indent spaces. */ - lineLen = (int)XSNPRINTF(line, lineLen - 1, "%*s", indent, " "); - /* Write indents string to BIO */ - if (wolfSSL_BIO_write(bio, line, lineLen) <= 0) { + int len_wanted = XSNPRINTF(line, lineLen, "%*s", indent, " "); + if (len_wanted >= lineLen) { + WOLFSSL_MSG("Buffer overflow formatting indentation"); ret = 0; + } else { + /* Write indents string to BIO */ + if (wolfSSL_BIO_write(bio, line, len_wanted) <= 0) { + ret = 0; + } } } @@ -193,11 +198,16 @@ static int wolfssl_print_value(WOLFSSL_BIO* bio, mp_int* value, /* Get 32-bits of value. */ v = (word32)value->dp[0]; /* Print the line to the string. */ - len = (int)XSNPRINTF(line, sizeof(line) - 1, "%s %u (0x%x)\n", name, v, + len = (int)XSNPRINTF(line, sizeof(line), "%s %u (0x%x)\n", name, v, v); - /* Write string to BIO */ - if (wolfSSL_BIO_write(bio, line, len) <= 0) { + if (len >= (int)sizeof(line)) { + WOLFSSL_MSG("Buffer overflow while formatting value"); ret = 0; + } else { + /* Write string to BIO */ + if (wolfSSL_BIO_write(bio, line, len) <= 0) { + ret = 0; + } } } @@ -247,9 +257,14 @@ static int wolfssl_print_number(WOLFSSL_BIO* bio, mp_int* num, const char* name, } if (ret == 1) { /* Print header string line to string. */ - li = XSNPRINTF(line, sizeof(line) - 1, "%s\n", name); - if (wolfSSL_BIO_write(bio, line, li) <= 0) { + li = XSNPRINTF(line, sizeof(line), "%s\n", name); + if (li >= (int)sizeof(line)) { + WOLFSSL_MSG("Buffer overflow formatting name"); ret = 0; + } else { + if (wolfSSL_BIO_write(bio, line, li) <= 0) { + ret = 0; + } } } if (ret == 1) { @@ -259,16 +274,27 @@ static int wolfssl_print_number(WOLFSSL_BIO* bio, mp_int* num, const char* name, if (ret == 1) { /* Start first digit line with spaces. * Writing out zeros ensures number is a positive value. */ - li = XSNPRINTF(line, sizeof(line) - 1, PRINT_NUM_INDENT "%s", + li = XSNPRINTF(line, sizeof(line), PRINT_NUM_INDENT "%s", mp_leading_bit(num) ? "00:" : ""); + if (li >= (int)sizeof(line)) { + WOLFSSL_MSG("Buffer overflow formatting spaces"); + ret = 0; + } } /* Put out each line of numbers. */ for (i = 0; (ret == 1) && (i < rawLen); i++) { - /* Check for a complete line. */ - if (li >= PRINT_NUM_MAX_DIGIT_LINE) { - /* More bytes coming so change the trailing ':' to a line break. */ - line[li-1] = '\n'; + /* Encode another byte as 2 hex digits and append colon. */ + int len_wanted = XSNPRINTF(line + li, sizeof(line) - li, "%02x:", + rawKey[i]); + /* Check if there was room -- if not, print the current line, not + * including the newest octet. + */ + if (len_wanted >= (int)sizeof(line) - li) { + /* bump current octet to the next line. */ + --i; + /* More bytes coming so add a line break. */ + line[li++] = '\n'; /* Write out the line. */ if (wolfSSL_BIO_write(bio, line, li) <= 0) { ret = 0; @@ -280,9 +306,8 @@ static int wolfssl_print_number(WOLFSSL_BIO* bio, mp_int* num, const char* name, /* Put the leading spaces on new line. */ XSTRNCPY(line, PRINT_NUM_INDENT, PRINT_NUM_INDENT_CNT + 1); li = PRINT_NUM_INDENT_CNT; - } - /* Encode another byte as 2 hex digits and append colon. */ - li += XSNPRINTF(line + li, sizeof(line) - 1 - li, "%02x:", rawKey[i]); + } else + li += len_wanted; } if (ret == 1) { @@ -1892,10 +1917,15 @@ int wolfSSL_RSA_print(WOLFSSL_BIO* bio, WOLFSSL_RSA* rsa, int indent) } if (ret == 1) { /* Print header line. */ - len = XSNPRINTF(line, sizeof(line) - 1, "\nRSA %s: (%d bit)\n", + len = XSNPRINTF(line, sizeof(line), "\nRSA %s: (%d bit)\n", (!mp_iszero(&key->d)) ? "Private-Key" : "Public-Key", sz); - if (wolfSSL_BIO_write(bio, line, len) <= 0) { + if (len >= (int)sizeof(line)) { + WOLFSSL_MSG("Buffer overflow while formatting key preamble"); ret = 0; + } else { + if (wolfSSL_BIO_write(bio, line, len) <= 0) { + ret = 0; + } } } From 4f6527353b58a574c37a5b27a43719e2d030baf6 Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Thu, 30 Jun 2022 16:31:25 -0500 Subject: [PATCH 5/6] src/{pk.c,x509.c}: style/clarity cleanups from dgarske. --- src/pk.c | 13 +++++++++---- src/x509.c | 6 ++++-- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/pk.c b/src/pk.c index 903b652a7..06de3d5e0 100644 --- a/src/pk.c +++ b/src/pk.c @@ -155,7 +155,8 @@ static int wolfssl_print_indent(WOLFSSL_BIO* bio, char* line, int lineLen, if (len_wanted >= lineLen) { WOLFSSL_MSG("Buffer overflow formatting indentation"); ret = 0; - } else { + } + else { /* Write indents string to BIO */ if (wolfSSL_BIO_write(bio, line, len_wanted) <= 0) { ret = 0; @@ -261,7 +262,8 @@ static int wolfssl_print_number(WOLFSSL_BIO* bio, mp_int* num, const char* name, if (li >= (int)sizeof(line)) { WOLFSSL_MSG("Buffer overflow formatting name"); ret = 0; - } else { + } + else { if (wolfSSL_BIO_write(bio, line, li) <= 0) { ret = 0; } @@ -306,8 +308,10 @@ static int wolfssl_print_number(WOLFSSL_BIO* bio, mp_int* num, const char* name, /* Put the leading spaces on new line. */ XSTRNCPY(line, PRINT_NUM_INDENT, PRINT_NUM_INDENT_CNT + 1); li = PRINT_NUM_INDENT_CNT; - } else + } + else { li += len_wanted; + } } if (ret == 1) { @@ -1922,7 +1926,8 @@ int wolfSSL_RSA_print(WOLFSSL_BIO* bio, WOLFSSL_RSA* rsa, int indent) if (len >= (int)sizeof(line)) { WOLFSSL_MSG("Buffer overflow while formatting key preamble"); ret = 0; - } else { + } + else { if (wolfSSL_BIO_write(bio, line, len) <= 0) { ret = 0; } diff --git a/src/x509.c b/src/x509.c index 27efc1280..7b52cf44f 100644 --- a/src/x509.c +++ b/src/x509.c @@ -6244,10 +6244,12 @@ int wolfSSL_X509_signature_print(WOLFSSL_BIO *bp, return WOLFSSL_FAILURE; } - if ((sigalg->algorithm->obj == NULL) || (sigalg->algorithm->obj[idx++] != ASN_OBJECT_ID)) { + if ((sigalg->algorithm->obj == NULL) || + (sigalg->algorithm->obj[idx] != ASN_OBJECT_ID)) { WOLFSSL_MSG("Bad ASN1 Object"); return WOLFSSL_FAILURE; } + idx++; /* skip object id */ if (GetLength((const byte*)sigalg->algorithm->obj, &idx, &length, sigalg->algorithm->objSz) < 0 || length < 0) { @@ -6262,7 +6264,7 @@ int wolfSSL_X509_signature_print(WOLFSSL_BIO *bp, for (i = 0; i < length; ++i) { char hex_digits[4]; #ifdef XSNPRINTF - XSNPRINTF(hex_digits, sizeof hex_digits, "%c%02X", i>0 ? ':' : ' ', + XSNPRINTF(hex_digits, sizeof(hex_digits), "%c%02X", i>0 ? ':' : ' ', (unsigned int)sigalg->algorithm->obj[idx+i]); #else XSPRINTF(hex_digits, "%c%02X", i>0 ? ':' : ' ', From 2bdcbcc8be1101eeecc685989545afe82a1ffd27 Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Thu, 30 Jun 2022 17:17:50 -0500 Subject: [PATCH 6/6] src/tls13.c: fix whitespace. --- src/tls13.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tls13.c b/src/tls13.c index e893fa753..431c2e666 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -1963,7 +1963,7 @@ static int EncryptTls13(WOLFSSL* ssl, byte* output, const byte* input, if (ret != CRYPTOCB_UNAVAILABLE) { if (ret > 0) { ret = 0; /* tsip_Tls13AesEncrypt returns output size */ - } + } return ret; } ret = 0;