1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
|
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
Upstream-Status: Backport from [https://github.com/curl/curl/commit/ed5095ed94281989e103c72e032200b83be37878 & https://github.com/curl/curl/commit/f18af4f874cecab82a9797e8c7541e0990c7a64c]
Comment: to backport fix for CVE-2023-27535, add function Curl_timestrcmp.
Signed-off-by: Vijay Anusuri <vanusuri@mvista.com>
---
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 | 21 ++++++++++++++++++++-
6 files changed, 61 insertions(+), 26 deletions(-)
diff --git a/lib/netrc.c b/lib/netrc.c
index 9323913..fe3fd1e 100644
--- a/lib/netrc.c
+++ b/lib/netrc.c
@@ -124,9 +124,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;
@@ -142,7 +142,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 70bf21c..ec776b3 100644
--- a/lib/strcase.c
+++ b/lib/strcase.c
@@ -261,6 +261,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 8929a53..8077108 100644
--- a/lib/strcase.h
+++ b/lib/strcase.h
@@ -49,5 +49,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 9f14a7b..dfbde3b 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -886,19 +886,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://tools.ietf.org/html/rfc3986#section-3.2.1 */
- if((data->user == NULL) != (needle->user == NULL))
- 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 == NULL) != (needle->passwd == NULL))
- 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;
}
@@ -1257,10 +1248,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;
}
@@ -1326,8 +1317,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 */
@@ -1348,8 +1339,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 a109056..3986386 100644
--- a/lib/vauth/digest_sspi.c
+++ b/lib/vauth/digest_sspi.c
@@ -450,8 +450,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 e8cb70f..70a9391 100644
--- a/lib/vtls/vtls.c
+++ b/lib/vtls/vtls.c
@@ -98,9 +98,15 @@ Curl_ssl_config_matches(struct ssl_primary_config* data,
Curl_safecmp(data->issuercert, needle->issuercert) &&
Curl_safecmp(data->clientcert, needle->clientcert) &&
Curl_safecmp(data->random_file, needle->random_file) &&
- Curl_safecmp(data->egdsocket, needle->egdsocket) &&
+ Curl_safecmp(data->egdsocket, needle->egdsocket) &&
+#ifdef USE_TLS_SRP
+ !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) &&
Curl_safe_strcasecompare(data->cipher_list13, needle->cipher_list13) &&
+ Curl_safe_strcasecompare(data->CRLfile, needle->CRLfile) &&
Curl_safe_strcasecompare(data->pinned_key, needle->pinned_key))
return TRUE;
@@ -117,6 +123,9 @@ Curl_clone_primary_ssl_config(struct ssl_primary_config *source,
dest->verifyhost = source->verifyhost;
dest->verifystatus = source->verifystatus;
dest->sessionid = source->sessionid;
+#ifdef USE_TLS_SRP
+ dest->authtype = source->authtype;
+#endif
CLONE_STRING(CApath);
CLONE_STRING(CAfile);
@@ -127,6 +136,11 @@ Curl_clone_primary_ssl_config(struct ssl_primary_config *source,
CLONE_STRING(cipher_list);
CLONE_STRING(cipher_list13);
CLONE_STRING(pinned_key);
+ CLONE_STRING(CRLfile);
+#ifdef USE_TLS_SRP
+ CLONE_STRING(username);
+ CLONE_STRING(password);
+#endif
return TRUE;
}
@@ -142,6 +156,11 @@ void Curl_free_primary_ssl_config(struct ssl_primary_config* sslc)
Curl_safefree(sslc->cipher_list);
Curl_safefree(sslc->cipher_list13);
Curl_safefree(sslc->pinned_key);
+ Curl_safefree(sslc->CRLfile);
+#ifdef USE_TLS_SRP
+ Curl_safefree(sslc->username);
+ Curl_safefree(sslc->password);
+#endif
}
#ifdef USE_SSL
--
2.25.1
|
