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
This commit is contained in:
Daniel Stenberg 2023-12-11 16:15:57 +01:00
parent 9efdefe6b1
commit ff74cef5d4
No known key found for this signature in database
GPG Key ID: 5CC908FDB71E12C2
7 changed files with 101 additions and 98 deletions

View File

@ -247,7 +247,7 @@ static CURLcode make_headers(struct Curl_easy *data,
} }
else { else {
char *value; char *value;
char *endp;
value = strchr(*date_header, ':'); value = strchr(*date_header, ':');
if(!value) { if(!value) {
*date_header = NULL; *date_header = NULL;
@ -256,8 +256,17 @@ static CURLcode make_headers(struct Curl_easy *data,
++value; ++value;
while(ISBLANK(*value)) while(ISBLANK(*value))
++value; ++value;
strncpy(timestamp, value, TIMESTAMP_SIZE - 1); endp = value;
timestamp[TIMESTAMP_SIZE - 1] = 0; 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; *date_header = NULL;
} }
@ -605,7 +614,7 @@ CURLcode Curl_output_aws_sigv4(struct Curl_easy *data, bool proxy)
result = CURLE_URL_MALFORMAT; result = CURLE_URL_MALFORMAT;
goto fail; goto fail;
} }
strncpy(service, hostname, len); memcpy(service, hostname, len);
service[len] = '\0'; service[len] = '\0';
infof(data, "aws_sigv4: picked service %s from host", service); 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; result = CURLE_URL_MALFORMAT;
goto fail; goto fail;
} }
strncpy(region, reg, len); memcpy(region, reg, len);
region[len] = '\0'; region[len] = '\0';
infof(data, "aws_sigv4: picked region %s from host", region); infof(data, "aws_sigv4: picked region %s from host", region);
} }

View File

@ -572,13 +572,15 @@ curl_url_strerror(CURLUcode error)
* Returns NULL if no error message was found for error code. * Returns NULL if no error message was found for error code.
*/ */
static const char * 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 #ifndef CURL_DISABLE_VERBOSE_STRINGS
const char *p; const char *p;
#endif #endif
if(!len) /* 41 bytes is the longest error string */
DEBUGASSERT(len > 41);
if(!len || len < 41)
return NULL; return NULL;
*buf = '\0'; *buf = '\0';
@ -755,8 +757,8 @@ get_winsock_error (int err, char *buf, size_t len)
default: default:
return NULL; return NULL;
} }
strncpy(buf, p, len); memcpy(buf, p, len - 1);
buf [len-1] = '\0'; buf[len - 1] = '\0';
return buf; return buf;
#endif #endif
} }
@ -832,7 +834,6 @@ const char *Curl_strerror(int err, char *buf, size_t buflen)
#endif #endif
int old_errno = errno; int old_errno = errno;
char *p; char *p;
size_t max;
if(!buflen) if(!buflen)
return NULL; return NULL;
@ -841,23 +842,22 @@ const char *Curl_strerror(int err, char *buf, size_t buflen)
DEBUGASSERT(err >= 0); DEBUGASSERT(err >= 0);
#endif #endif
max = buflen - 1;
*buf = '\0'; *buf = '\0';
#if defined(_WIN32) || defined(_WIN32_WCE) #if defined(_WIN32) || defined(_WIN32_WCE)
#if defined(_WIN32) #if defined(_WIN32)
/* 'sys_nerr' is the maximum errno number, it is not widely portable */ /* 'sys_nerr' is the maximum errno number, it is not widely portable */
if(err >= 0 && err < sys_nerr) if(err >= 0 && err < sys_nerr)
strncpy(buf, sys_errlist[err], max); msnprintf(buf, buflen, "%s", sys_errlist[err]);
else else
#endif #endif
{ {
if( if(
#ifdef USE_WINSOCK #ifdef USE_WINSOCK
!get_winsock_error(err, buf, max) && !get_winsock_error(err, buf, buflen) &&
#endif #endif
!get_winapi_error((DWORD)err, buf, max)) !get_winapi_error((DWORD)err, buf, buflen))
msnprintf(buf, max, "Unknown error %d (%#x)", err, err); msnprintf(buf, buflen, "Unknown error %d (%#x)", err, err);
} }
#else /* not Windows coming up */ #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 * storage is supplied via 'strerrbuf' and 'buflen' to hold the generated
* message string, or EINVAL if 'errnum' is not a valid error number. * 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]) 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) #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 buffer[256];
char *msg = strerror_r(err, buffer, sizeof(buffer)); char *msg = strerror_r(err, buffer, sizeof(buffer));
if(msg) if(msg)
strncpy(buf, msg, max); msnprintf(buf, buflen, "%s", msg);
else else
msnprintf(buf, max, "Unknown error %d", err); msnprintf(buf, buflen, "Unknown error %d", err);
} }
#else #else
{ {
/* !checksrc! disable STRERROR 1 */ /* !checksrc! disable STRERROR 1 */
const char *msg = strerror(err); const char *msg = strerror(err);
if(msg) if(msg)
strncpy(buf, msg, max); msnprintf(buf, buflen, "%s", msg);
else else
msnprintf(buf, max, "Unknown error %d", err); msnprintf(buf, buflen, "Unknown error %d", err);
} }
#endif #endif
#endif /* end of not Windows */ #endif /* end of not Windows */
buf[max] = '\0'; /* make sure the string is null-terminated */
/* strip trailing '\r\n' or '\n'. */ /* strip trailing '\r\n' or '\n'. */
p = strrchr(buf, '\n'); p = strrchr(buf, '\n');
if(p && (p - buf) >= 2) if(p && (p - buf) >= 2)
@ -943,8 +941,8 @@ const char *Curl_winapi_strerror(DWORD err, char *buf, size_t buflen)
#else #else
{ {
const char *txt = (err == ERROR_SUCCESS) ? "No error" : "Error"; const char *txt = (err == ERROR_SUCCESS) ? "No error" : "Error";
strncpy(buf, txt, buflen); if(strlen(txt) < buflen)
buf[buflen - 1] = '\0'; strcpy(buf, txt);
} }
#endif #endif
@ -1081,17 +1079,11 @@ const char *Curl_sspi_strerror(int err, char *buf, size_t buflen)
err); err);
} }
else { else {
char txtbuf[80];
char msgbuf[256]; char msgbuf[256];
msnprintf(txtbuf, sizeof(txtbuf), "%s (0x%08X)", txt, err);
if(get_winapi_error(err, msgbuf, sizeof(msgbuf))) if(get_winapi_error(err, msgbuf, sizeof(msgbuf)))
msnprintf(buf, buflen, "%s - %s", txtbuf, msgbuf); msnprintf(buf, buflen, "%s (0x%08X) - %s", txt, err, msgbuf);
else { else
strncpy(buf, txtbuf, buflen); msnprintf(buf, buflen, "%s (0x%08X)", txt, err);
buf[buflen - 1] = '\0';
}
} }
#else #else
@ -1099,8 +1091,8 @@ const char *Curl_sspi_strerror(int err, char *buf, size_t buflen)
txt = "No error"; txt = "No error";
else else
txt = "Error"; txt = "Error";
strncpy(buf, txt, buflen); if(buflen > strlen(txt))
buf[buflen - 1] = '\0'; strcpy(buf, txt);
#endif #endif
if(errno != old_errno) if(errno != old_errno)

View File

@ -826,23 +826,27 @@ static CURLcode check_telnet_options(struct Curl_easy *data)
case 5: case 5:
/* Terminal type */ /* Terminal type */
if(strncasecompare(option, "TTYPE", 5)) { if(strncasecompare(option, "TTYPE", 5)) {
strncpy(tn->subopt_ttype, arg, 31); size_t l = strlen(arg);
tn->subopt_ttype[31] = 0; /* String termination */ if(l < sizeof(tn->subopt_ttype)) {
tn->us_preferred[CURL_TELOPT_TTYPE] = CURL_YES; strcpy(tn->subopt_ttype, arg);
tn->us_preferred[CURL_TELOPT_TTYPE] = CURL_YES;
break;
}
} }
else result = CURLE_UNKNOWN_OPTION;
result = CURLE_UNKNOWN_OPTION;
break; break;
case 8: case 8:
/* Display variable */ /* Display variable */
if(strncasecompare(option, "XDISPLOC", 8)) { if(strncasecompare(option, "XDISPLOC", 8)) {
strncpy(tn->subopt_xdisploc, arg, 127); size_t l = strlen(arg);
tn->subopt_xdisploc[127] = 0; /* String termination */ if(l < sizeof(tn->subopt_xdisploc)) {
tn->us_preferred[CURL_TELOPT_XDISPLOC] = CURL_YES; strcpy(tn->subopt_xdisploc, arg);
tn->us_preferred[CURL_TELOPT_XDISPLOC] = CURL_YES;
break;
}
} }
else result = CURLE_UNKNOWN_OPTION;
result = CURLE_UNKNOWN_OPTION;
break; break;
case 7: case 7:

View File

@ -509,7 +509,6 @@ static CURLcode bearssl_set_selected_ciphers(struct Curl_easy *data,
{ {
uint16_t selected_ciphers[NUM_OF_CIPHERS]; uint16_t selected_ciphers[NUM_OF_CIPHERS];
size_t selected_count = 0; size_t selected_count = 0;
char cipher_name[CIPHER_NAME_BUF_LEN];
const char *cipher_start = ciphers; const char *cipher_start = ciphers;
const char *cipher_end; const char *cipher_end;
size_t i, j; size_t i, j;
@ -518,41 +517,48 @@ static CURLcode bearssl_set_selected_ciphers(struct Curl_easy *data,
return CURLE_SSL_CIPHER; return CURLE_SSL_CIPHER;
while(true) { while(true) {
const char *cipher;
size_t clen;
/* Extract the next cipher name from the ciphers string */ /* Extract the next cipher name from the ciphers string */
while(is_separator(*cipher_start)) while(is_separator(*cipher_start))
++cipher_start; ++cipher_start;
if(*cipher_start == '\0') if(!*cipher_start)
break; break;
cipher_end = cipher_start; cipher_end = cipher_start;
while(*cipher_end != '\0' && !is_separator(*cipher_end)) while(*cipher_end && !is_separator(*cipher_end))
++cipher_end; ++cipher_end;
j = cipher_end - cipher_start < CIPHER_NAME_BUF_LEN - 1 ?
cipher_end - cipher_start : CIPHER_NAME_BUF_LEN - 1; clen = cipher_end - cipher_start;
strncpy(cipher_name, cipher_start, j); cipher = cipher_start;
cipher_name[j] = '\0';
cipher_start = cipher_end; cipher_start = cipher_end;
/* Lookup the cipher name in the table of available ciphers. If the cipher /* 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 name starts with "TLS_" we do the lookup by IANA name. Otherwise, we try
to match cipher name by an (OpenSSL) alias. */ 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 && 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 { else {
for(i = 0; i < NUM_OF_CIPHERS && 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) { 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; continue;
} }
/* No duplicates allowed */ /* No duplicates allowed */
for(j = 0; j < selected_count && for(j = 0; j < selected_count &&
selected_ciphers[j] != ciphertable[i].num; j++); selected_ciphers[j] != ciphertable[i].num; j++);
if(j < selected_count) { 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; continue;
} }

View File

@ -954,8 +954,9 @@ static char *ossl_strerror(unsigned long error, char *buf, size_t size)
#endif #endif
if(!*buf) { if(!*buf) {
strncpy(buf, (error ? "Unknown error" : "No error"), size); const char *msg = error ? "Unknown error" : "No error";
buf[size - 1] = '\0'; if(strlen(msg) < size)
strcpy(buf, msg);
} }
return buf; return buf;
@ -4592,10 +4593,10 @@ static ssize_t ossl_send(struct Curl_cfilter *cf,
ossl_strerror(sslerror, error_buffer, sizeof(error_buffer)); ossl_strerror(sslerror, error_buffer, sizeof(error_buffer));
else if(sockerr) else if(sockerr)
Curl_strerror(sockerr, error_buffer, sizeof(error_buffer)); Curl_strerror(sockerr, error_buffer, sizeof(error_buffer));
else { else
strncpy(error_buffer, SSL_ERROR_to_str(err), sizeof(error_buffer)); msnprintf(error_buffer, sizeof(error_buffer), "%s",
error_buffer[sizeof(error_buffer) - 1] = '\0'; SSL_ERROR_to_str(err));
}
failf(data, OSSL_PACKAGE " SSL_write: %s, errno %d", failf(data, OSSL_PACKAGE " SSL_write: %s, errno %d",
error_buffer, sockerr); error_buffer, sockerr);
*curlcode = CURLE_SEND_ERROR; *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)); ossl_strerror(sslerror, error_buffer, sizeof(error_buffer));
else if(sockerr && err == SSL_ERROR_SYSCALL) else if(sockerr && err == SSL_ERROR_SYSCALL)
Curl_strerror(sockerr, error_buffer, sizeof(error_buffer)); Curl_strerror(sockerr, error_buffer, sizeof(error_buffer));
else { else
strncpy(error_buffer, SSL_ERROR_to_str(err), sizeof(error_buffer)); msnprintf(error_buffer, sizeof(error_buffer), "%s",
error_buffer[sizeof(error_buffer) - 1] = '\0'; SSL_ERROR_to_str(err));
}
failf(data, OSSL_PACKAGE " SSL_read: %s, errno %d", failf(data, OSSL_PACKAGE " SSL_read: %s, errno %d",
error_buffer, sockerr); error_buffer, sockerr);
*curlcode = CURLE_RECV_ERROR; *curlcode = CURLE_RECV_ERROR;

View File

@ -439,6 +439,12 @@ get_cert_location(TCHAR *path, DWORD *store_name, TCHAR **store_path,
return CURLE_OK; return CURLE_OK;
} }
#endif #endif
static bool algo(const char *check, char *namep, size_t nlen)
{
return (strlen(check) == nlen) && !strncmp(check, namep, nlen);
}
static CURLcode static CURLcode
schannel_acquire_credential_handle(struct Curl_cfilter *cf, schannel_acquire_credential_handle(struct Curl_cfilter *cf,
struct Curl_easy *data) struct Curl_easy *data)
@ -790,9 +796,7 @@ schannel_acquire_credential_handle(struct Curl_cfilter *cf,
char *startCur = ciphers13; char *startCur = ciphers13;
int algCount = 0; int algCount = 0;
char tmp[LONGEST_ALG_ID] = { 0 };
char *nameEnd; char *nameEnd;
size_t n;
disable_aes_gcm_sha384 = TRUE; disable_aes_gcm_sha384 = TRUE;
disable_aes_gcm_sha256 = TRUE; disable_aes_gcm_sha256 = TRUE;
@ -801,40 +805,34 @@ schannel_acquire_credential_handle(struct Curl_cfilter *cf,
disable_aes_ccm_sha256 = TRUE; disable_aes_ccm_sha256 = TRUE;
while(startCur && (0 != *startCur) && (algCount < remaining_ciphers)) { while(startCur && (0 != *startCur) && (algCount < remaining_ciphers)) {
size_t n;
char *namep;
nameEnd = strchr(startCur, ':'); nameEnd = strchr(startCur, ':');
n = nameEnd ? (size_t)(nameEnd - startCur) : strlen(startCur); n = nameEnd ? (size_t)(nameEnd - startCur) : strlen(startCur);
namep = startCur;
/* reject too-long cipher names */ if(disable_aes_gcm_sha384 &&
if(n > (LONGEST_ALG_ID - 1)) { algo("TLS_AES_256_GCM_SHA384", namep, n)) {
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)) {
disable_aes_gcm_sha384 = FALSE; disable_aes_gcm_sha384 = FALSE;
} }
else if(disable_aes_gcm_sha256 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; disable_aes_gcm_sha256 = FALSE;
} }
else if(disable_chacha_poly else if(disable_chacha_poly
&& !strcmp("TLS_CHACHA20_POLY1305_SHA256", tmp)) { && algo("TLS_CHACHA20_POLY1305_SHA256", namep, n)) {
disable_chacha_poly = FALSE; disable_chacha_poly = FALSE;
} }
else if(disable_aes_ccm_8_sha256 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; disable_aes_ccm_8_sha256 = FALSE;
} }
else if(disable_aes_ccm_sha256 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; disable_aes_ccm_sha256 = FALSE;
} }
else { 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; return CURLE_SSL_CIPHER;
} }

View File

@ -1413,17 +1413,11 @@ static size_t multissl_version(char *buffer, size_t size)
backends_len = p - backends; backends_len = p - backends;
} }
if(!size) if(size && (size < backends_len))
return 0; strcpy(buffer, backends);
else
if(size <= backends_len) { *buffer = 0; /* did not fit */
strncpy(buffer, backends, size - 1); return 0;
buffer[size - 1] = '\0';
return size - 1;
}
strcpy(buffer, backends);
return backends_len;
} }
static int multissl_setup(const struct Curl_ssl *backend) static int multissl_setup(const struct Curl_ssl *backend)