From 295124c256ed25f097192cfa9a67e460f7bb587f Mon Sep 17 00:00:00 2001 From: nao Date: Tue, 21 Jan 2020 10:30:37 +0100 Subject: [PATCH 1/2] http: move "oauth_bearer" from connectdata to Curl_easy Fixes the bug where oauth_bearer gets deallocated when we re-use a connection. Closes #4824 Upstream-commit: dea17b519dc1d83265ca6aa9a484a2cf242db3b9 Signed-off-by: Kamil Dudka --- lib/curl_sasl.c | 14 ++++++++------ lib/http.c | 12 +++++------- lib/url.c | 9 --------- lib/urldata.h | 2 -- 4 files changed, 13 insertions(+), 24 deletions(-) diff --git a/lib/curl_sasl.c b/lib/curl_sasl.c index 354bc54..c767bef 100644 --- a/lib/curl_sasl.c +++ b/lib/curl_sasl.c @@ -269,6 +269,7 @@ CURLcode Curl_sasl_start(struct SASL *sasl, struct connectdata *conn, data->set.str[STRING_SERVICE_NAME] : sasl->params->service; #endif + const char *oauth_bearer = data->set.str[STRING_BEARER]; sasl->force_ir = force_ir; /* Latch for future use */ sasl->authused = 0; /* No mechanism used yet */ @@ -339,7 +340,7 @@ CURLcode Curl_sasl_start(struct SASL *sasl, struct connectdata *conn, } else #endif - if((enabledmechs & SASL_MECH_OAUTHBEARER) && conn->oauth_bearer) { + if((enabledmechs & SASL_MECH_OAUTHBEARER) && oauth_bearer) { mech = SASL_MECH_STRING_OAUTHBEARER; state1 = SASL_OAUTH2; state2 = SASL_OAUTH2_RESP; @@ -349,10 +350,10 @@ CURLcode Curl_sasl_start(struct SASL *sasl, struct connectdata *conn, result = Curl_auth_create_oauth_bearer_message(data, conn->user, hostname, port, - conn->oauth_bearer, + oauth_bearer, &resp, &len); } - else if((enabledmechs & SASL_MECH_XOAUTH2) && conn->oauth_bearer) { + else if((enabledmechs & SASL_MECH_XOAUTH2) && oauth_bearer) { mech = SASL_MECH_STRING_XOAUTH2; state1 = SASL_OAUTH2; sasl->authused = SASL_MECH_XOAUTH2; @@ -360,7 +361,7 @@ CURLcode Curl_sasl_start(struct SASL *sasl, struct connectdata *conn, if(force_ir || data->set.sasl_ir) result = Curl_auth_create_oauth_bearer_message(data, conn->user, NULL, 0, - conn->oauth_bearer, + oauth_bearer, &resp, &len); } else if(enabledmechs & SASL_MECH_PLAIN) { @@ -429,6 +430,7 @@ CURLcode Curl_sasl_continue(struct SASL *sasl, struct connectdata *conn, char *serverdata; #endif size_t len = 0; + const char *oauth_bearer = data->set.str[STRING_BEARER]; *progress = SASL_INPROGRESS; @@ -556,7 +558,7 @@ CURLcode Curl_sasl_continue(struct SASL *sasl, struct connectdata *conn, result = Curl_auth_create_oauth_bearer_message(data, conn->user, hostname, port, - conn->oauth_bearer, + oauth_bearer, &resp, &len); /* Failures maybe sent by the server as continuations for OAUTHBEARER */ @@ -565,7 +567,7 @@ CURLcode Curl_sasl_continue(struct SASL *sasl, struct connectdata *conn, else result = Curl_auth_create_oauth_bearer_message(data, conn->user, NULL, 0, - conn->oauth_bearer, + oauth_bearer, &resp, &len); break; diff --git a/lib/http.c b/lib/http.c index 26eb52d..bf19077 100644 --- a/lib/http.c +++ b/lib/http.c @@ -326,7 +326,7 @@ static CURLcode http_output_bearer(struct connectdata *conn) userp = &conn->allocptr.userpwd; free(*userp); *userp = aprintf("Authorization: Bearer %s\r\n", - conn->oauth_bearer); + conn->data->set.str[STRING_BEARER]); if(!*userp) { result = CURLE_OUT_OF_MEMORY; @@ -510,7 +510,7 @@ CURLcode Curl_http_auth_act(struct connectdata *conn) CURLcode result = CURLE_OK; unsigned long authmask = ~0ul; - if(!conn->oauth_bearer) + if(!data->set.str[STRING_BEARER]) authmask &= (unsigned long)~CURLAUTH_BEARER; if(100 <= data->req.httpcode && 199 >= data->req.httpcode) @@ -520,7 +520,7 @@ CURLcode Curl_http_auth_act(struct connectdata *conn) if(data->state.authproblem) return data->set.http_fail_on_error?CURLE_HTTP_RETURNED_ERROR:CURLE_OK; - if((conn->bits.user_passwd || conn->oauth_bearer) && + if((conn->bits.user_passwd || data->set.str[STRING_BEARER]) && ((data->req.httpcode == 401) || (conn->bits.authneg && data->req.httpcode < 300))) { pickhost = pickoneauth(&data->state.authhost, authmask); @@ -590,9 +590,7 @@ output_auth_headers(struct connectdata *conn, { const char *auth = NULL; CURLcode result = CURLE_OK; -#if !defined(CURL_DISABLE_VERBOSE_STRINGS) || defined(USE_SPNEGO) struct Curl_easy *data = conn->data; -#endif #ifdef USE_SPNEGO struct negotiatedata *negdata = proxy ? &data->state.proxyneg : &data->state.negotiate; @@ -664,7 +662,7 @@ output_auth_headers(struct connectdata *conn, } if(authstatus->picked == CURLAUTH_BEARER) { /* Bearer */ - if((!proxy && conn->oauth_bearer && + if((!proxy && data->set.str[STRING_BEARER] && !Curl_checkheaders(conn, "Authorization:"))) { auth = "Bearer"; result = http_output_bearer(conn); @@ -722,7 +720,7 @@ Curl_http_output_auth(struct connectdata *conn, authproxy = &data->state.authproxy; if((conn->bits.httpproxy && conn->bits.proxy_user_passwd) || - conn->bits.user_passwd || conn->oauth_bearer) + conn->bits.user_passwd || data->set.str[STRING_BEARER]) /* continue please */; else { authhost->done = TRUE; diff --git a/lib/url.c b/lib/url.c index 4803653..fca0855 100644 --- a/lib/url.c +++ b/lib/url.c @@ -686,7 +686,6 @@ static void conn_free(struct connectdata *conn) Curl_safefree(conn->user); Curl_safefree(conn->passwd); - Curl_safefree(conn->oauth_bearer); Curl_safefree(conn->options); Curl_safefree(conn->http_proxy.user); Curl_safefree(conn->socks_proxy.user); @@ -4161,14 +4160,6 @@ static CURLcode create_conn(struct Curl_easy *data, } } - if(data->set.str[STRING_BEARER]) { - conn->oauth_bearer = strdup(data->set.str[STRING_BEARER]); - if(!conn->oauth_bearer) { - result = CURLE_OUT_OF_MEMORY; - goto out; - } - } - #ifdef USE_UNIX_SOCKETS if(data->set.str[STRING_UNIX_SOCKET_PATH]) { conn->unix_domain_socket = strdup(data->set.str[STRING_UNIX_SOCKET_PATH]); diff --git a/lib/urldata.h b/lib/urldata.h index 72a36fb..73a185c 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -850,8 +850,6 @@ struct connectdata { char *passwd; /* password string, allocated */ char *options; /* options string, allocated */ - char *oauth_bearer; /* bearer token for OAuth 2.0, allocated */ - int httpversion; /* the HTTP version*10 reported by the server */ int rtspversion; /* the RTSP version*10 reported by the server */ -- 2.34.1 From 85d1103c2fc0c9b1bdfae470dbafd45758e1c2f0 Mon Sep 17 00:00:00 2001 From: Patrick Monnerat Date: Mon, 25 Apr 2022 11:44:05 +0200 Subject: [PATCH 2/2] url: check sasl additional parameters for connection reuse. Also move static function safecmp() as non-static Curl_safecmp() since its purpose is needed at several places. Bug: https://curl.se/docs/CVE-2022-22576.html CVE-2022-22576 Closes #8746 Upstream-commit: 852aa5ad351ea53e5f01d2f44b5b4370c2bf5425 Signed-off-by: Kamil Dudka --- lib/strcase.c | 10 ++++++++++ lib/strcase.h | 2 ++ lib/url.c | 12 +++++++++++- lib/urldata.h | 2 ++ lib/vtls/vtls.c | 19 +++++-------------- 5 files changed, 30 insertions(+), 15 deletions(-) diff --git a/lib/strcase.c b/lib/strcase.c index dd46ca1..692a3f1 100644 --- a/lib/strcase.c +++ b/lib/strcase.c @@ -165,6 +165,16 @@ void Curl_strntoupper(char *dest, const char *src, size_t n) } while(*src++ && --n); } +/* Compare case-sensitive NUL-terminated strings, taking care of possible + * null pointers. Return true if arguments match. + */ +bool Curl_safecmp(char *a, char *b) +{ + if(a && b) + return !strcmp(a, b); + return !a && !b; +} + /* --- public functions --- */ int curl_strequal(const char *first, const char *second) diff --git a/lib/strcase.h b/lib/strcase.h index b628656..382b80a 100644 --- a/lib/strcase.h +++ b/lib/strcase.h @@ -47,4 +47,6 @@ char Curl_raw_toupper(char in); void Curl_strntoupper(char *dest, const char *src, size_t n); +bool Curl_safecmp(char *a, char *b); + #endif /* HEADER_CURL_STRCASE_H */ diff --git a/lib/url.c b/lib/url.c index adef2cd..94e3406 100644 --- a/lib/url.c +++ b/lib/url.c @@ -701,6 +701,7 @@ static void conn_free(struct connectdata *conn) Curl_safefree(conn->allocptr.host); Curl_safefree(conn->allocptr.cookiehost); Curl_safefree(conn->allocptr.rtsp_transport); + Curl_safefree(conn->oauth_bearer); Curl_safefree(conn->trailer); Curl_safefree(conn->host.rawalloc); /* host name buffer */ Curl_safefree(conn->conn_to_host.rawalloc); /* host name buffer */ @@ -1291,7 +1292,8 @@ ConnectionExists(struct Curl_easy *data, /* 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)) { + strcmp(needle->passwd, check->passwd) || + !Curl_safecmp(needle->oauth_bearer, check->oauth_bearer)) { /* one of them was different */ continue; } @@ -4160,6 +4162,14 @@ static CURLcode create_conn(struct Curl_easy *data, } } + if(data->set.str[STRING_BEARER]) { + conn->oauth_bearer = strdup(data->set.str[STRING_BEARER]); + if(!conn->oauth_bearer) { + result = CURLE_OUT_OF_MEMORY; + goto out; + } + } + #ifdef USE_UNIX_SOCKETS if(data->set.str[STRING_UNIX_SOCKET_PATH]) { conn->unix_domain_socket = strdup(data->set.str[STRING_UNIX_SOCKET_PATH]); diff --git a/lib/urldata.h b/lib/urldata.h index cc8a600..03da59a 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -850,6 +850,8 @@ struct connectdata { char *passwd; /* password string, allocated */ char *options; /* options string, allocated */ + char *oauth_bearer; /* OAUTH2 bearer, allocated */ + int httpversion; /* the HTTP version*10 reported by the server */ int rtspversion; /* the RTSP version*10 reported by the server */ diff --git a/lib/vtls/vtls.c b/lib/vtls/vtls.c index 03b85ba..a40ac06 100644 --- a/lib/vtls/vtls.c +++ b/lib/vtls/vtls.c @@ -82,15 +82,6 @@ else \ dest->var = NULL; -static bool safecmp(char *a, char *b) -{ - if(a && b) - return !strcmp(a, b); - else if(!a && !b) - return TRUE; /* match */ - return FALSE; /* no match */ -} - bool Curl_ssl_config_matches(struct ssl_primary_config* data, struct ssl_primary_config* needle) @@ -100,11 +91,11 @@ Curl_ssl_config_matches(struct ssl_primary_config* data, (data->verifypeer == needle->verifypeer) && (data->verifyhost == needle->verifyhost) && (data->verifystatus == needle->verifystatus) && - safecmp(data->CApath, needle->CApath) && - safecmp(data->CAfile, needle->CAfile) && - safecmp(data->clientcert, needle->clientcert) && - safecmp(data->random_file, needle->random_file) && - safecmp(data->egdsocket, needle->egdsocket) && + Curl_safecmp(data->CApath, needle->CApath) && + Curl_safecmp(data->CAfile, needle->CAfile) && + Curl_safecmp(data->clientcert, needle->clientcert) && + Curl_safecmp(data->random_file, needle->random_file) && + Curl_safecmp(data->egdsocket, needle->egdsocket) && Curl_safe_strcasecompare(data->cipher_list, needle->cipher_list) && Curl_safe_strcasecompare(data->cipher_list13, needle->cipher_list13)) return TRUE; -- 2.34.1