Backport of: From ed5095ed94281989e103c72e032200b83be37878 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg <daniel@haxx.se> 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 --- lib/netrc.c | 6 +++--- lib/strcase.c | 22 ++++++++++++++++++++++ lib/strcase.h | 1 + lib/url.c | 34 +++++++++++++--------------------- lib/vauth/digest_sspi.c | 4 ++-- lib/vtls/vtls.c | 4 ++-- 6 files changed, 43 insertions(+), 28 deletions(-) --- a/lib/netrc.c +++ b/lib/netrc.c @@ -123,9 +123,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; @@ -141,7 +141,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; --- a/lib/strcase.c +++ b/lib/strcase.c @@ -259,6 +259,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) --- a/lib/strcase.h +++ b/lib/strcase.h @@ -50,5 +50,6 @@ void Curl_strntoupper(char *dest, const 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 */ --- a/lib/url.c +++ b/lib/url.c @@ -1224,10 +1224,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; } @@ -1293,8 +1293,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 */ @@ -1315,8 +1315,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) { --- a/lib/vauth/digest_sspi.c +++ b/lib/vauth/digest_sspi.c @@ -450,8 +450,8 @@ CURLcode Curl_auth_create_digest_http_me 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); --- a/lib/vtls/vtls.c +++ b/lib/vtls/vtls.c @@ -109,8 +109,8 @@ Curl_ssl_config_matches(struct ssl_prima 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) &&