From 573d51966a2ec257dcdc20b8182f2f1c23d57c8a Mon Sep 17 00:00:00 2001 From: Kareem Abuobeid Date: Fri, 30 Apr 2021 12:46:44 -0700 Subject: [PATCH] S/MIME: Canonicalize multi-part messages before hashing. Improve error checking in wc_MIME_parse_headers. --- src/ssl.c | 45 ++++++++++++++++++++++++-------- wolfcrypt/src/asn.c | 57 ++++++++++++++++++++++++++++++++++++++--- wolfssl/wolfcrypt/asn.h | 1 + 3 files changed, 89 insertions(+), 14 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index d1116648c..f7a67c66d 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -54354,11 +54354,14 @@ WOLFSSL_API PKCS7* wolfSSL_SMIME_read_PKCS7(WOLFSSL_BIO* in, int sectionLen = 0; int ret = -1; char* section = NULL; + char* canonLine = NULL; + char* canonSection = NULL; PKCS7* pkcs7 = NULL; word32 outLen = 0; byte* out = NULL; byte* outHead = NULL; + int canonPos = 0; int lineLen = 0; int remainLen = 0; byte isEnd = 0; @@ -54461,38 +54464,60 @@ WOLFSSL_API PKCS7* wolfSSL_SMIME_read_PKCS7(WOLFSSL_BIO* in, section[0] = '\0'; sectionLen = 0; + canonSection = XMALLOC((remainLen+1)*sizeof(char), NULL, + DYNAMIC_TYPE_PKCS7); + if (canonSection == NULL) { + goto error; + } + lineLen = wolfSSL_BIO_gets(in, section, remainLen); while(XSTRNCMP(§ion[sectionLen], boundary, boundLen) && remainLen > 0) { + canonLine = wc_MIME_canonicalize(§ion[sectionLen]); + if (canonLine == NULL) { + XFREE(canonSection, NULL, DYNAMIC_TYPE_PKCS7); + goto error; + } + XMEMCPY(&canonSection[canonPos], canonLine, + (int)XSTRLEN(canonLine)); + canonPos += XSTRLEN(canonLine); + XFREE(canonLine, NULL, DYNAMIC_TYPE_PKCS7); + sectionLen += lineLen; remainLen -= lineLen; + lineLen = wolfSSL_BIO_gets(in, §ion[sectionLen], remainLen); if (lineLen <= 0) { + XFREE(canonSection, NULL, DYNAMIC_TYPE_PKCS7); goto error; } } - sectionLen--; + + canonPos--; /* Strip the final trailing newline. Support \r, \n or \r\n. */ - if (section[sectionLen] == '\n') { - sectionLen--; - if (section[sectionLen] == '\r') { - sectionLen--; + if (canonSection[canonPos] == '\n') { + canonPos--; + if (canonSection[canonPos] == '\r') { + canonPos--; } } - else if (section[sectionLen] == '\r') { - sectionLen--; + else if (canonSection[canonPos] == '\r') { + canonPos--; } - section[sectionLen+1] = '\0'; + canonSection[canonPos+1] = '\0'; *bcont = wolfSSL_BIO_new(wolfSSL_BIO_s_mem()); - ret = wolfSSL_BIO_write(*bcont, section, (int)XSTRLEN(section)); - if (ret != (int)XSTRLEN(section)) { + ret = wolfSSL_BIO_write(*bcont, canonSection, + (int)XSTRLEN(canonSection)); + if (ret != (int)XSTRLEN(canonSection)) { + XFREE(canonSection, NULL, DYNAMIC_TYPE_PKCS7); goto error; } if ((bcontMemSz = wolfSSL_BIO_get_mem_data(*bcont, &bcontMem)) < 0) { goto error; } + XFREE(canonSection, NULL, DYNAMIC_TYPE_PKCS7); wc_MIME_free_hdrs(allHdrs); diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 010be3f53..718ec2162 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -18547,13 +18547,17 @@ int wc_MIME_parse_headers(char* in, int inLen, MimeHdr** headers) } start = end = 0; lineLen = XSTRLEN(curLine); + if (lineLen == 0) { + ret = BAD_FUNC_ARG; + goto error; + } for (pos = 0; pos < lineLen; pos++) { char cur = curLine[pos]; if (mimeStatus == MIME_NAMEATTR && ((cur == ':' && mimeType == MIME_HDR) || (cur == '=' && - mimeType == MIME_PARAM))) { + mimeType == MIME_PARAM)) && pos >= 1) { mimeStatus = MIME_BODYVAL; end = pos-1; ret = wc_MIME_header_strip(curLine, &nameAttr, start, end); @@ -18562,7 +18566,7 @@ int wc_MIME_parse_headers(char* in, int inLen, MimeHdr** headers) } start = pos+1; } - else if (mimeStatus == MIME_BODYVAL && cur == ';') { + else if (mimeStatus == MIME_BODYVAL && cur == ';' && pos >= 1) { end = pos-1; ret = wc_MIME_header_strip(curLine, &bodyVal, start, end); if (ret) { @@ -18570,7 +18574,9 @@ int wc_MIME_parse_headers(char* in, int inLen, MimeHdr** headers) } if (mimeType == MIME_HDR) { nextHdr->name = nameAttr; + nameAttr = NULL; nextHdr->body = bodyVal; + bodyVal = NULL; nextHdr->next = curHdr; curHdr = nextHdr; nextHdr = (MimeHdr*)XMALLOC(sizeof(MimeHdr), NULL, @@ -18583,7 +18589,9 @@ int wc_MIME_parse_headers(char* in, int inLen, MimeHdr** headers) } else { nextParam->attribute = nameAttr; + nameAttr = NULL; nextParam->value = bodyVal; + bodyVal = NULL; nextParam->next = curHdr->params; curHdr->params = nextParam; nextParam = (MimeParam*)XMALLOC(sizeof(MimeParam), NULL, @@ -18612,7 +18620,9 @@ int wc_MIME_parse_headers(char* in, int inLen, MimeHdr** headers) } if (mimeType == MIME_HDR) { nextHdr->name = nameAttr; + nameAttr = NULL; nextHdr->body = bodyVal; + bodyVal = NULL; nextHdr->next = curHdr; curHdr = nextHdr; nextHdr = (MimeHdr*)XMALLOC(sizeof(MimeHdr), NULL, @@ -18624,7 +18634,9 @@ int wc_MIME_parse_headers(char* in, int inLen, MimeHdr** headers) XMEMSET(nextHdr, 0, (word32)sizeof(MimeHdr)); } else { nextParam->attribute = nameAttr; + nameAttr = NULL; nextParam->value = bodyVal; + bodyVal = NULL; nextParam->next = curHdr->params; curHdr->params = nextParam; nextParam = (MimeParam*)XMALLOC(sizeof(MimeParam), NULL, @@ -18650,8 +18662,10 @@ int wc_MIME_parse_headers(char* in, int inLen, MimeHdr** headers) error: wc_MIME_free_hdrs(curHdr); wc_MIME_free_hdrs(nextHdr); - XFREE(nameAttr, NULL, DYNAMIC_TYPE_PKCS7); - XFREE(bodyVal, NULL, DYNAMIC_TYPE_PKCS7); + if (nameAttr != NULL) + XFREE(nameAttr, NULL, DYNAMIC_TYPE_PKCS7); + if (bodyVal != NULL) + XFREE(bodyVal, NULL, DYNAMIC_TYPE_PKCS7); XFREE(nextParam, NULL, DYNAMIC_TYPE_PKCS7); return ret; @@ -18742,6 +18756,41 @@ MimeParam* wc_MIME_find_param_attr(const char* attribute, return param; } +/***************************************************************************** +* wc_MIME_canonicalize - Canonicalize a line by converting all line endings +* to CRLF. +* +* RETURNS: +* returns a pointer to a canonicalized line on success, NULL on error. +*/ +char* wc_MIME_canonicalize(const char* line) +{ + size_t end = 0; + char* canonLine = NULL; + + if (line == NULL || XSTRLEN(line) == 0) { + return NULL; + } + + end = XSTRLEN(line); + while (end >= 1 && ((line[end-1] == '\r') || (line[end-1] == '\n'))) { + end--; + } + + /* Need 2 chars for \r\n and 1 for EOL */ + canonLine = (char*)XMALLOC((end+3)*sizeof(char), NULL, DYNAMIC_TYPE_PKCS7); + if (canonLine == NULL) { + return NULL; + } + + XSTRNCPY(canonLine, line, end); + canonLine[end] = '\r'; + canonLine[end+1] = '\n'; + canonLine[end+2] = '\0'; + + return canonLine; +} + /***************************************************************************** * wc_MIME_free_hdrs - Frees all MIME headers, parameters and strings starting from * the provided header pointer. diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index 4bfc55545..e7cb80f6c 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -1295,6 +1295,7 @@ WOLFSSL_LOCAL int wc_MIME_create_header(char* name, char* body, MimeHdr** hdr); WOLFSSL_LOCAL int wc_MIME_create_parameter(char* attribute, char* value, MimeParam** param); WOLFSSL_LOCAL MimeHdr* wc_MIME_find_header_name(const char* name, MimeHdr* hdr); WOLFSSL_LOCAL MimeParam* wc_MIME_find_param_attr(const char* attribute, MimeParam* param); +WOLFSSL_LOCAL char* wc_MIME_canonicalize(const char* line); WOLFSSL_LOCAL int wc_MIME_free_hdrs(MimeHdr* head); #endif /* HAVE_SMIME */