]> git.ipfire.org Git - thirdparty/curl.git/commitdiff
strcase: add and use Curl_timestrcmp
authorDaniel Stenberg <daniel@haxx.se>
Wed, 5 Oct 2022 22:49:10 +0000 (00:49 +0200)
committerDaniel Stenberg <daniel@haxx.se>
Sat, 8 Oct 2022 09:50:47 +0000 (11:50 +0200)
This is a strcmp() alternative function for comparing "secrets",
designed to take the same time no matter the content to not leak
match/non-match info to observers based on how fast it is.

The time this function takes is only a function of the shortest input
string.

Reported-by: Trail of Bits
Closes #9658

lib/netrc.c
lib/strcase.c
lib/strcase.h
lib/url.c
lib/vauth/digest_sspi.c
lib/vtls/vtls.c

index e183b6271de35aab3edaf9a46860287343cdc993..f09c75842cdbd1afc5b8ccc8c32989bde8c00cab 100644 (file)
@@ -198,9 +198,9 @@ static int parsenetrc(const char *host,
           /* we are now parsing sub-keywords concerning "our" host */
           if(state_login) {
             if(specific_login) {
-              state_our_login = !strcmp(login, tok);
+              state_our_login = !Curl_timestrcmp(login, tok);
             }
-            else if(!login || strcmp(login, tok)) {
+            else if(!login || Curl_timestrcmp(login, tok)) {
               if(login_alloc) {
                 free(login);
                 login_alloc = FALSE;
@@ -216,7 +216,7 @@ static int parsenetrc(const char *host,
           }
           else if(state_password) {
             if((state_our_login || !specific_login)
-                && (!password || strcmp(password, tok))) {
+               && (!password || Curl_timestrcmp(password, tok))) {
               if(password_alloc) {
                 free(password);
                 password_alloc = FALSE;
index 7f49e78687ae79a280d43b302a0a2f89b06eb855..09d2a8a9610d2a22085bd0763d79fe8a52a7dc95 100644 (file)
@@ -177,6 +177,28 @@ bool Curl_safecmp(char *a, char *b)
   return !a && !b;
 }
 
+/*
+ * Curl_timestrcmp() returns 0 if the two strings are identical. The time this
+ * function spends is a function of the shortest string, not of the contents.
+ */
+int Curl_timestrcmp(const char *a, const char *b)
+{
+  int match = 0;
+  int i = 0;
+
+  if(a && b) {
+    while(1) {
+      match |= a[i]^b[i];
+      if(!a[i] || !b[i])
+        break;
+      i++;
+    }
+  }
+  else
+    return a || b;
+  return match;
+}
+
 /* --- public functions --- */
 
 int curl_strequal(const char *first, const char *second)
index 8ebd42a07edfefdd5507d85b7c5e53452a9e2a9c..65a575385df771a4e7c2535b9c236da5976903d4 100644 (file)
@@ -53,5 +53,6 @@ void Curl_strntoupper(char *dest, const char *src, size_t n);
 void Curl_strntolower(char *dest, const char *src, size_t n);
 
 bool Curl_safecmp(char *a, char *b);
+int Curl_timestrcmp(const char *first, const char *second);
 
 #endif /* HEADER_CURL_STRCASE_H */
index 65eefb86e2e5961f26675d82b0378c03cd22394a..a10c0b1550a3328423defd0b37da564e77d607b5 100644 (file)
--- a/lib/url.c
+++ b/lib/url.c
@@ -957,21 +957,11 @@ socks_proxy_info_matches(const struct proxy_info *data,
   /* the user information is case-sensitive
      or at least it is not defined as case-insensitive
      see https://datatracker.ietf.org/doc/html/rfc3986#section-3.2.1 */
-  if(!data->user != !needle->user)
-    return FALSE;
-  /* curl_strequal does a case insensitive comparison,
-     so do not use it here! */
-  if(data->user &&
-     needle->user &&
-     strcmp(data->user, needle->user) != 0)
-    return FALSE;
-  if(!data->passwd != !needle->passwd)
-    return FALSE;
+
   /* curl_strequal does a case insensitive comparison,
      so do not use it here! */
-  if(data->passwd &&
-     needle->passwd &&
-     strcmp(data->passwd, needle->passwd) != 0)
+  if(Curl_timestrcmp(data->user, needle->user) ||
+     Curl_timestrcmp(data->passwd, needle->passwd))
     return FALSE;
   return TRUE;
 }
@@ -1373,10 +1363,10 @@ ConnectionExists(struct Curl_easy *data,
       if(!(needle->handler->flags & PROTOPT_CREDSPERREQUEST)) {
         /* This protocol requires credentials per connection,
            so verify that we're using the same name and password as well */
-        if(strcmp(needle->user, check->user) ||
-           strcmp(needle->passwd, check->passwd) ||
-           !Curl_safecmp(needle->sasl_authzid, check->sasl_authzid) ||
-           !Curl_safecmp(needle->oauth_bearer, check->oauth_bearer)) {
+        if(Curl_timestrcmp(needle->user, check->user) ||
+           Curl_timestrcmp(needle->passwd, check->passwd) ||
+           Curl_timestrcmp(needle->sasl_authzid, check->sasl_authzid) ||
+           Curl_timestrcmp(needle->oauth_bearer, check->oauth_bearer)) {
           /* one of them was different */
           continue;
         }
@@ -1452,8 +1442,8 @@ ConnectionExists(struct Curl_easy *data,
            possible. (Especially we must not reuse the same connection if
            partway through a handshake!) */
         if(wantNTLMhttp) {
-          if(strcmp(needle->user, check->user) ||
-             strcmp(needle->passwd, check->passwd)) {
+          if(Curl_timestrcmp(needle->user, check->user) ||
+             Curl_timestrcmp(needle->passwd, check->passwd)) {
 
             /* we prefer a credential match, but this is at least a connection
                that can be reused and "upgraded" to NTLM */
@@ -1475,8 +1465,10 @@ ConnectionExists(struct Curl_easy *data,
           if(!check->http_proxy.user || !check->http_proxy.passwd)
             continue;
 
-          if(strcmp(needle->http_proxy.user, check->http_proxy.user) ||
-             strcmp(needle->http_proxy.passwd, check->http_proxy.passwd))
+          if(Curl_timestrcmp(needle->http_proxy.user,
+                             check->http_proxy.user) ||
+             Curl_timestrcmp(needle->http_proxy.passwd,
+                             check->http_proxy.passwd))
             continue;
         }
         else if(check->proxy_ntlm_state != NTLMSTATE_NONE) {
index 66c85fa26bccb4c51a0c07cadecd0ab658301472..89a9db52c7b055f24c0e6f028ae4a3d771517696 100644 (file)
@@ -431,8 +431,8 @@ CURLcode Curl_auth_create_digest_http_message(struct Curl_easy *data,
      has changed then delete that context. */
   if((userp && !digest->user) || (!userp && digest->user) ||
      (passwdp && !digest->passwd) || (!passwdp && digest->passwd) ||
-     (userp && digest->user && strcmp(userp, digest->user)) ||
-     (passwdp && digest->passwd && strcmp(passwdp, digest->passwd))) {
+     (userp && digest->user && Curl_timestrcmp(userp, digest->user)) ||
+     (passwdp && digest->passwd && Curl_timestrcmp(passwdp, digest->passwd))) {
     if(digest->http_context) {
       s_pSecFn->DeleteSecurityContext(digest->http_context);
       Curl_safefree(digest->http_context);
index 8048ba7a4193c45179af4b6480f1f8f943dfa3cf..9dee5aa3b38ef77908343bc5954868e55fa9725e 100644 (file)
@@ -146,8 +146,8 @@ Curl_ssl_config_matches(struct ssl_primary_config *data,
      Curl_safecmp(data->issuercert, needle->issuercert) &&
      Curl_safecmp(data->clientcert, needle->clientcert) &&
 #ifdef USE_TLS_SRP
-     Curl_safecmp(data->username, needle->username) &&
-     Curl_safecmp(data->password, needle->password) &&
+     !Curl_timestrcmp(data->username, needle->username) &&
+     !Curl_timestrcmp(data->password, needle->password) &&
      (data->authtype == needle->authtype) &&
 #endif
      Curl_safe_strcasecompare(data->cipher_list, needle->cipher_list) &&