From ff74cef5d4a0cf60106517a1c7384ebec039e0a2 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Mon, 11 Dec 2023 16:15:57 +0100 Subject: [PATCH] lib: reduce use of strncpy - bearssl: select cipher without buffer copies - http_aws_sigv4: avoid strncpy, require exact timestamp length - http_aws_sigv4: use memcpy isntead of strncpy - openssl: avoid strncpy calls - schannel: check for 1.3 algos without buffer copies - strerror: avoid strncpy calls - telnet: avoid strncpy, return error on too long inputs - vtls: avoid strncpy in multissl_version() Closes #12499 --- lib/http_aws_sigv4.c | 19 ++++++++++++---- lib/strerror.c | 54 +++++++++++++++++++------------------------- lib/telnet.c | 24 ++++++++++++-------- lib/vtls/bearssl.c | 32 +++++++++++++++----------- lib/vtls/openssl.c | 20 ++++++++-------- lib/vtls/schannel.c | 34 +++++++++++++--------------- lib/vtls/vtls.c | 16 ++++--------- 7 files changed, 101 insertions(+), 98 deletions(-) diff --git a/lib/http_aws_sigv4.c b/lib/http_aws_sigv4.c index b673055f30..c9382918ed 100644 --- a/lib/http_aws_sigv4.c +++ b/lib/http_aws_sigv4.c @@ -247,7 +247,7 @@ static CURLcode make_headers(struct Curl_easy *data, } else { char *value; - + char *endp; value = strchr(*date_header, ':'); if(!value) { *date_header = NULL; @@ -256,8 +256,17 @@ static CURLcode make_headers(struct Curl_easy *data, ++value; while(ISBLANK(*value)) ++value; - strncpy(timestamp, value, TIMESTAMP_SIZE - 1); - timestamp[TIMESTAMP_SIZE - 1] = 0; + endp = value; + while(*endp && ISALNUM(*endp)) + ++endp; + /* 16 bytes => "19700101T000000Z" */ + if((endp - value) == TIMESTAMP_SIZE - 1) { + memcpy(timestamp, value, TIMESTAMP_SIZE - 1); + timestamp[TIMESTAMP_SIZE - 1] = 0; + } + else + /* bad timestamp length */ + timestamp[0] = 0; *date_header = NULL; } @@ -605,7 +614,7 @@ CURLcode Curl_output_aws_sigv4(struct Curl_easy *data, bool proxy) result = CURLE_URL_MALFORMAT; goto fail; } - strncpy(service, hostname, len); + memcpy(service, hostname, len); service[len] = '\0'; infof(data, "aws_sigv4: picked service %s from host", service); @@ -624,7 +633,7 @@ CURLcode Curl_output_aws_sigv4(struct Curl_easy *data, bool proxy) result = CURLE_URL_MALFORMAT; goto fail; } - strncpy(region, reg, len); + memcpy(region, reg, len); region[len] = '\0'; infof(data, "aws_sigv4: picked region %s from host", region); } diff --git a/lib/strerror.c b/lib/strerror.c index 0d5f9276f0..51e1485408 100644 --- a/lib/strerror.c +++ b/lib/strerror.c @@ -572,13 +572,15 @@ curl_url_strerror(CURLUcode error) * Returns NULL if no error message was found for error code. */ static const char * -get_winsock_error (int err, char *buf, size_t len) +get_winsock_error(int err, char *buf, size_t len) { #ifndef CURL_DISABLE_VERBOSE_STRINGS const char *p; #endif - if(!len) + /* 41 bytes is the longest error string */ + DEBUGASSERT(len > 41); + if(!len || len < 41) return NULL; *buf = '\0'; @@ -755,8 +757,8 @@ get_winsock_error (int err, char *buf, size_t len) default: return NULL; } - strncpy(buf, p, len); - buf [len-1] = '\0'; + memcpy(buf, p, len - 1); + buf[len - 1] = '\0'; return buf; #endif } @@ -832,7 +834,6 @@ const char *Curl_strerror(int err, char *buf, size_t buflen) #endif int old_errno = errno; char *p; - size_t max; if(!buflen) return NULL; @@ -841,23 +842,22 @@ const char *Curl_strerror(int err, char *buf, size_t buflen) DEBUGASSERT(err >= 0); #endif - max = buflen - 1; *buf = '\0'; #if defined(_WIN32) || defined(_WIN32_WCE) #if defined(_WIN32) /* 'sys_nerr' is the maximum errno number, it is not widely portable */ if(err >= 0 && err < sys_nerr) - strncpy(buf, sys_errlist[err], max); + msnprintf(buf, buflen, "%s", sys_errlist[err]); else #endif { if( #ifdef USE_WINSOCK - !get_winsock_error(err, buf, max) && + !get_winsock_error(err, buf, buflen) && #endif - !get_winapi_error((DWORD)err, buf, max)) - msnprintf(buf, max, "Unknown error %d (%#x)", err, err); + !get_winapi_error((DWORD)err, buf, buflen)) + msnprintf(buf, buflen, "Unknown error %d (%#x)", err, err); } #else /* not Windows coming up */ @@ -867,9 +867,9 @@ const char *Curl_strerror(int err, char *buf, size_t buflen) * storage is supplied via 'strerrbuf' and 'buflen' to hold the generated * message string, or EINVAL if 'errnum' is not a valid error number. */ - if(0 != strerror_r(err, buf, max)) { + if(0 != strerror_r(err, buf, buflen)) { if('\0' == buf[0]) - msnprintf(buf, max, "Unknown error %d", err); + msnprintf(buf, buflen, "Unknown error %d", err); } #elif defined(HAVE_STRERROR_R) && defined(HAVE_GLIBC_STRERROR_R) /* @@ -881,25 +881,23 @@ const char *Curl_strerror(int err, char *buf, size_t buflen) char buffer[256]; char *msg = strerror_r(err, buffer, sizeof(buffer)); if(msg) - strncpy(buf, msg, max); + msnprintf(buf, buflen, "%s", msg); else - msnprintf(buf, max, "Unknown error %d", err); + msnprintf(buf, buflen, "Unknown error %d", err); } #else { /* !checksrc! disable STRERROR 1 */ const char *msg = strerror(err); if(msg) - strncpy(buf, msg, max); + msnprintf(buf, buflen, "%s", msg); else - msnprintf(buf, max, "Unknown error %d", err); + msnprintf(buf, buflen, "Unknown error %d", err); } #endif #endif /* end of not Windows */ - buf[max] = '\0'; /* make sure the string is null-terminated */ - /* strip trailing '\r\n' or '\n'. */ p = strrchr(buf, '\n'); if(p && (p - buf) >= 2) @@ -943,8 +941,8 @@ const char *Curl_winapi_strerror(DWORD err, char *buf, size_t buflen) #else { const char *txt = (err == ERROR_SUCCESS) ? "No error" : "Error"; - strncpy(buf, txt, buflen); - buf[buflen - 1] = '\0'; + if(strlen(txt) < buflen) + strcpy(buf, txt); } #endif @@ -1081,17 +1079,11 @@ const char *Curl_sspi_strerror(int err, char *buf, size_t buflen) err); } else { - char txtbuf[80]; char msgbuf[256]; - - msnprintf(txtbuf, sizeof(txtbuf), "%s (0x%08X)", txt, err); - if(get_winapi_error(err, msgbuf, sizeof(msgbuf))) - msnprintf(buf, buflen, "%s - %s", txtbuf, msgbuf); - else { - strncpy(buf, txtbuf, buflen); - buf[buflen - 1] = '\0'; - } + msnprintf(buf, buflen, "%s (0x%08X) - %s", txt, err, msgbuf); + else + msnprintf(buf, buflen, "%s (0x%08X)", txt, err); } #else @@ -1099,8 +1091,8 @@ const char *Curl_sspi_strerror(int err, char *buf, size_t buflen) txt = "No error"; else txt = "Error"; - strncpy(buf, txt, buflen); - buf[buflen - 1] = '\0'; + if(buflen > strlen(txt)) + strcpy(buf, txt); #endif if(errno != old_errno) diff --git a/lib/telnet.c b/lib/telnet.c index 836e255c9d..6c689c8166 100644 --- a/lib/telnet.c +++ b/lib/telnet.c @@ -826,23 +826,27 @@ static CURLcode check_telnet_options(struct Curl_easy *data) case 5: /* Terminal type */ if(strncasecompare(option, "TTYPE", 5)) { - strncpy(tn->subopt_ttype, arg, 31); - tn->subopt_ttype[31] = 0; /* String termination */ - tn->us_preferred[CURL_TELOPT_TTYPE] = CURL_YES; + size_t l = strlen(arg); + if(l < sizeof(tn->subopt_ttype)) { + strcpy(tn->subopt_ttype, arg); + tn->us_preferred[CURL_TELOPT_TTYPE] = CURL_YES; + break; + } } - else - result = CURLE_UNKNOWN_OPTION; + result = CURLE_UNKNOWN_OPTION; break; case 8: /* Display variable */ if(strncasecompare(option, "XDISPLOC", 8)) { - strncpy(tn->subopt_xdisploc, arg, 127); - tn->subopt_xdisploc[127] = 0; /* String termination */ - tn->us_preferred[CURL_TELOPT_XDISPLOC] = CURL_YES; + size_t l = strlen(arg); + if(l < sizeof(tn->subopt_xdisploc)) { + strcpy(tn->subopt_xdisploc, arg); + tn->us_preferred[CURL_TELOPT_XDISPLOC] = CURL_YES; + break; + } } - else - result = CURLE_UNKNOWN_OPTION; + result = CURLE_UNKNOWN_OPTION; break; case 7: diff --git a/lib/vtls/bearssl.c b/lib/vtls/bearssl.c index a6566f4d90..58394bab9a 100644 --- a/lib/vtls/bearssl.c +++ b/lib/vtls/bearssl.c @@ -509,7 +509,6 @@ static CURLcode bearssl_set_selected_ciphers(struct Curl_easy *data, { uint16_t selected_ciphers[NUM_OF_CIPHERS]; size_t selected_count = 0; - char cipher_name[CIPHER_NAME_BUF_LEN]; const char *cipher_start = ciphers; const char *cipher_end; size_t i, j; @@ -518,41 +517,48 @@ static CURLcode bearssl_set_selected_ciphers(struct Curl_easy *data, return CURLE_SSL_CIPHER; while(true) { + const char *cipher; + size_t clen; + /* Extract the next cipher name from the ciphers string */ while(is_separator(*cipher_start)) ++cipher_start; - if(*cipher_start == '\0') + if(!*cipher_start) break; cipher_end = cipher_start; - while(*cipher_end != '\0' && !is_separator(*cipher_end)) + while(*cipher_end && !is_separator(*cipher_end)) ++cipher_end; - j = cipher_end - cipher_start < CIPHER_NAME_BUF_LEN - 1 ? - cipher_end - cipher_start : CIPHER_NAME_BUF_LEN - 1; - strncpy(cipher_name, cipher_start, j); - cipher_name[j] = '\0'; + + clen = cipher_end - cipher_start; + cipher = cipher_start; + cipher_start = cipher_end; /* Lookup the cipher name in the table of available ciphers. If the cipher name starts with "TLS_" we do the lookup by IANA name. Otherwise, we try to match cipher name by an (OpenSSL) alias. */ - if(strncasecompare(cipher_name, "TLS_", 4)) { + if(strncasecompare(cipher, "TLS_", 4)) { for(i = 0; i < NUM_OF_CIPHERS && - !strcasecompare(cipher_name, ciphertable[i].name); ++i); + (strlen(ciphertable[i].name) == clen) && + !strncasecompare(cipher, ciphertable[i].name, clen); ++i); } else { for(i = 0; i < NUM_OF_CIPHERS && - !strcasecompare(cipher_name, ciphertable[i].alias_name); ++i); + (strlen(ciphertable[i].alias_name) == clen) && + !strncasecompare(cipher, ciphertable[i].alias_name, clen); ++i); } if(i == NUM_OF_CIPHERS) { - infof(data, "BearSSL: unknown cipher in list: %s", cipher_name); + infof(data, "BearSSL: unknown cipher in list: %.*s", + (int)clen, cipher); continue; } /* No duplicates allowed */ for(j = 0; j < selected_count && - selected_ciphers[j] != ciphertable[i].num; j++); + selected_ciphers[j] != ciphertable[i].num; j++); if(j < selected_count) { - infof(data, "BearSSL: duplicate cipher in list: %s", cipher_name); + infof(data, "BearSSL: duplicate cipher in list: %.*s", + (int)clen, cipher); continue; } diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c index 8c8f43e836..1f735e0014 100644 --- a/lib/vtls/openssl.c +++ b/lib/vtls/openssl.c @@ -954,8 +954,9 @@ static char *ossl_strerror(unsigned long error, char *buf, size_t size) #endif if(!*buf) { - strncpy(buf, (error ? "Unknown error" : "No error"), size); - buf[size - 1] = '\0'; + const char *msg = error ? "Unknown error" : "No error"; + if(strlen(msg) < size) + strcpy(buf, msg); } return buf; @@ -4592,10 +4593,10 @@ static ssize_t ossl_send(struct Curl_cfilter *cf, ossl_strerror(sslerror, error_buffer, sizeof(error_buffer)); else if(sockerr) Curl_strerror(sockerr, error_buffer, sizeof(error_buffer)); - else { - strncpy(error_buffer, SSL_ERROR_to_str(err), sizeof(error_buffer)); - error_buffer[sizeof(error_buffer) - 1] = '\0'; - } + else + msnprintf(error_buffer, sizeof(error_buffer), "%s", + SSL_ERROR_to_str(err)); + failf(data, OSSL_PACKAGE " SSL_write: %s, errno %d", error_buffer, sockerr); *curlcode = CURLE_SEND_ERROR; @@ -4688,10 +4689,9 @@ static ssize_t ossl_recv(struct Curl_cfilter *cf, ossl_strerror(sslerror, error_buffer, sizeof(error_buffer)); else if(sockerr && err == SSL_ERROR_SYSCALL) Curl_strerror(sockerr, error_buffer, sizeof(error_buffer)); - else { - strncpy(error_buffer, SSL_ERROR_to_str(err), sizeof(error_buffer)); - error_buffer[sizeof(error_buffer) - 1] = '\0'; - } + else + msnprintf(error_buffer, sizeof(error_buffer), "%s", + SSL_ERROR_to_str(err)); failf(data, OSSL_PACKAGE " SSL_read: %s, errno %d", error_buffer, sockerr); *curlcode = CURLE_RECV_ERROR; diff --git a/lib/vtls/schannel.c b/lib/vtls/schannel.c index 8e1d5be8d8..b2a707950d 100644 --- a/lib/vtls/schannel.c +++ b/lib/vtls/schannel.c @@ -439,6 +439,12 @@ get_cert_location(TCHAR *path, DWORD *store_name, TCHAR **store_path, return CURLE_OK; } #endif + +static bool algo(const char *check, char *namep, size_t nlen) +{ + return (strlen(check) == nlen) && !strncmp(check, namep, nlen); +} + static CURLcode schannel_acquire_credential_handle(struct Curl_cfilter *cf, struct Curl_easy *data) @@ -790,9 +796,7 @@ schannel_acquire_credential_handle(struct Curl_cfilter *cf, char *startCur = ciphers13; int algCount = 0; - char tmp[LONGEST_ALG_ID] = { 0 }; char *nameEnd; - size_t n; disable_aes_gcm_sha384 = TRUE; disable_aes_gcm_sha256 = TRUE; @@ -801,40 +805,34 @@ schannel_acquire_credential_handle(struct Curl_cfilter *cf, disable_aes_ccm_sha256 = TRUE; while(startCur && (0 != *startCur) && (algCount < remaining_ciphers)) { + size_t n; + char *namep; nameEnd = strchr(startCur, ':'); n = nameEnd ? (size_t)(nameEnd - startCur) : strlen(startCur); + namep = startCur; - /* reject too-long cipher names */ - if(n > (LONGEST_ALG_ID - 1)) { - failf(data, "schannel: Cipher name too long, not checked"); - return CURLE_SSL_CIPHER; - } - - strncpy(tmp, startCur, n); - tmp[n] = 0; - - if(disable_aes_gcm_sha384 - && !strcmp("TLS_AES_256_GCM_SHA384", tmp)) { + if(disable_aes_gcm_sha384 && + algo("TLS_AES_256_GCM_SHA384", namep, n)) { disable_aes_gcm_sha384 = FALSE; } else if(disable_aes_gcm_sha256 - && !strcmp("TLS_AES_128_GCM_SHA256", tmp)) { + && algo("TLS_AES_128_GCM_SHA256", namep, n)) { disable_aes_gcm_sha256 = FALSE; } else if(disable_chacha_poly - && !strcmp("TLS_CHACHA20_POLY1305_SHA256", tmp)) { + && algo("TLS_CHACHA20_POLY1305_SHA256", namep, n)) { disable_chacha_poly = FALSE; } else if(disable_aes_ccm_8_sha256 - && !strcmp("TLS_AES_128_CCM_8_SHA256", tmp)) { + && algo("TLS_AES_128_CCM_8_SHA256", namep, n)) { disable_aes_ccm_8_sha256 = FALSE; } else if(disable_aes_ccm_sha256 - && !strcmp("TLS_AES_128_CCM_SHA256", tmp)) { + && algo("TLS_AES_128_CCM_SHA256", namep, n)) { disable_aes_ccm_sha256 = FALSE; } else { - failf(data, "schannel: Unknown TLS 1.3 cipher: %s", tmp); + failf(data, "schannel: Unknown TLS 1.3 cipher: %.*s", (int)n, namep); return CURLE_SSL_CIPHER; } diff --git a/lib/vtls/vtls.c b/lib/vtls/vtls.c index dd68a06636..256b8faa80 100644 --- a/lib/vtls/vtls.c +++ b/lib/vtls/vtls.c @@ -1413,17 +1413,11 @@ static size_t multissl_version(char *buffer, size_t size) backends_len = p - backends; } - if(!size) - return 0; - - if(size <= backends_len) { - strncpy(buffer, backends, size - 1); - buffer[size - 1] = '\0'; - return size - 1; - } - - strcpy(buffer, backends); - return backends_len; + if(size && (size < backends_len)) + strcpy(buffer, backends); + else + *buffer = 0; /* did not fit */ + return 0; } static int multissl_setup(const struct Curl_ssl *backend) -- 2.47.3