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

diff --git a/lib/http_aws_sigv4.c b/lib/http_aws_sigv4.c
index b673055f3097b8047eed6585cc59b083179e0ca4..c9382918ed6f928363adfb6e3231d5ca5e63e836 100644 (file)
--- 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 0d5f9276f03c72b28b5567b8b8eba8f21ca6684d..51e1485408e83b72c5e9d5252b6d8e1d486658ce 100644 (file)
--- 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 836e255c9d6304e07412893089e10e84ac3cced4..6c689c81662394b608cb268385d539642951da63 100644 (file)
--- 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 a6566f4d90c8753a37473fe57e07c25f41db1c51..58394bab9a63425e4ee8fb175e7589702190b222 100644 (file)
--- 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 8c8f43e8360f0215aae667bb0a3c59021e0c0da4..1f735e00144a66a10dafc9cd5c1f4690d2af4bf3 100644 (file)
--- 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 8e1d5be8d8025c74d12a34378596518cb0666363..b2a707950d566366893bd04110c6ab829340c332 100644 (file)
--- 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 dd68a066367ce5e2661b0001413d6567a422a630..256b8faa804f99556886baff49a0e88247da3ca6 100644 (file)
--- 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)