From ed5095ed94281989e103c72e032200b83be37878 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Thu, 6 Oct 2022 00:49:10 +0200 Subject: [PATCH] strcase: add and use Curl_timestrcmp 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 Upstream-Status: Backport from [https://github.com/curl/curl/commit/ed5095ed94281989e103c72e032200b83be37878] Comment: to backport fix for CVE-2023-27535, add function Curl_timestrcmp. Signed-off-by: Siddharth Doshi --- lib/netrc.c | 6 +++--- lib/strcase.c | 22 ++++++++++++++++++++++ lib/strcase.h | 1 + lib/url.c | 33 +++++++++++++-------------------- lib/vauth/digest_sspi.c | 4 ++-- lib/vtls/vtls.c | 4 ++-- 6 files changed, 43 insertions(+), 27 deletions(-) diff --git a/lib/netrc.c b/lib/netrc.c index 0a4ae2c..b771b60 100644 --- a/lib/netrc.c +++ b/lib/netrc.c @@ -140,9 +140,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 = strcasecompare(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; @@ -158,7 +158,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; diff --git a/lib/strcase.c b/lib/strcase.c index 692a3f1..be085b3 100644 --- a/lib/strcase.c +++ b/lib/strcase.c @@ -141,6 +141,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) diff --git a/lib/strcase.h b/lib/strcase.h index 382b80a..c6979da 100644 --- a/lib/strcase.h +++ b/lib/strcase.h @@ -48,5 +48,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 */ diff --git a/lib/url.c b/lib/url.c index df4377d..c397b57 100644 --- a/lib/url.c +++ b/lib/url.c @@ -930,19 +930,10 @@ 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 insentive 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 insentive 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; } @@ -1341,10 +1332,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; } @@ -1420,8 +1411,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 */ @@ -1443,8 +1434,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) { diff --git a/lib/vauth/digest_sspi.c b/lib/vauth/digest_sspi.c index 94f8f8c..a413419 100644 --- a/lib/vauth/digest_sspi.c +++ b/lib/vauth/digest_sspi.c @@ -429,8 +429,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); diff --git a/lib/vtls/vtls.c b/lib/vtls/vtls.c index e2d3438..881c8d2 100644 --- a/lib/vtls/vtls.c +++ b/lib/vtls/vtls.c @@ -146,8 +146,8 @@ Curl_ssl_config_matches(struct ssl_primary_config *data, Curl_safecmp(data->random_file, needle->random_file) && Curl_safecmp(data->egdsocket, needle->egdsocket) && #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) && -- 2.35.7