From 9bee39bfed2c413b4cc4eb306a57ac92a1854907 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Sat, 12 Oct 2024 23:54:39 +0200 Subject: [PATCH] url: use same credentials on redirect Previously it could lose the username and only use the password. Added test 998 and 999 to verify. Reported-by: Tobias Bora Fixes #15262 Closes #15282 Changes: - Test files are added in Makefile.inc. CVE: CVE-2024-11053 Upstream-Status: Backport [https://github.com/curl/curl/commit/9bee39bfed2c413b4cc4eb306a57ac92a1854907] Signed-off-by: Yogita Urade --- lib/transfer.c | 3 ++ lib/url.c | 19 +++++---- lib/urldata.h | 9 +++- tests/data/Makefile.inc | 2 +- tests/data/test998 | 92 +++++++++++++++++++++++++++++++++++++++++ tests/data/test999 | 81 ++++++++++++++++++++++++++++++++++++ 6 files changed, 195 insertions(+), 11 deletions(-) create mode 100644 tests/data/test998 create mode 100644 tests/data/test999 diff --git a/lib/transfer.c b/lib/transfer.c index e31d1d6..ccd042b 100644 --- a/lib/transfer.c +++ b/lib/transfer.c @@ -700,6 +700,9 @@ CURLcode Curl_pretransfer(struct Curl_easy *data) return CURLE_OUT_OF_MEMORY; } + if(data->set.str[STRING_USERNAME] || + data->set.str[STRING_PASSWORD]) + data->state.creds_from = CREDS_OPTION; if(!result) result = Curl_setstropt(&data->state.aptr.user, data->set.str[STRING_USERNAME]); diff --git a/lib/url.c b/lib/url.c index 224b9f3..05431b9 100644 --- a/lib/url.c +++ b/lib/url.c @@ -1899,10 +1899,10 @@ static CURLcode parseurlandfillconn(struct Curl_easy *data, return result; /* - * User name and password set with their own options override the - * credentials possibly set in the URL. + * username and password set with their own options override the credentials + * possibly set in the URL, but netrc does not. */ - if(!data->set.str[STRING_PASSWORD]) { + if(!data->state.aptr.passwd || (data->state.creds_from != CREDS_OPTION)) { uc = curl_url_get(uh, CURLUPART_PASSWORD, &data->state.up.password, 0); if(!uc) { char *decoded; @@ -1915,12 +1915,13 @@ static CURLcode parseurlandfillconn(struct Curl_easy *data, result = Curl_setstropt(&data->state.aptr.passwd, decoded); if(result) return result; + data->state.creds_from = CREDS_URL; } else if(uc != CURLUE_NO_PASSWORD) return Curl_uc_to_curlcode(uc); } - if(!data->set.str[STRING_USERNAME]) { + if(!data->state.aptr.user || (data->state.creds_from != CREDS_OPTION)) { /* we don't use the URL API's URL decoder option here since it rejects control codes and we want to allow them for some schemes in the user and password fields */ @@ -1934,13 +1935,10 @@ static CURLcode parseurlandfillconn(struct Curl_easy *data, return result; conn->user = decoded; result = Curl_setstropt(&data->state.aptr.user, decoded); + data->state.creds_from = CREDS_URL; } else if(uc != CURLUE_NO_USER) return Curl_uc_to_curlcode(uc); - else if(data->state.aptr.passwd) { - /* no user was set but a password, set a blank user */ - result = Curl_setstropt(&data->state.aptr.user, ""); - } if(result) return result; } @@ -2730,7 +2728,8 @@ static CURLcode override_login(struct Curl_easy *data, int ret; bool url_provided = FALSE; - if(data->state.aptr.user) { + if(data->state.aptr.user && + (data->state.creds_from != CREDS_NETRC)) { /* there was a user name in the URL. Use the URL decoded version */ userp = &data->state.aptr.user; url_provided = TRUE; @@ -2778,6 +2777,7 @@ static CURLcode override_login(struct Curl_easy *data, result = Curl_setstropt(&data->state.aptr.user, *userp); if(result) return result; + data->state.creds_from = CREDS_NETRC; } } if(data->state.aptr.user) { @@ -2795,6 +2795,7 @@ static CURLcode override_login(struct Curl_easy *data, CURLcode result = Curl_setstropt(&data->state.aptr.passwd, *passwdp); if(result) return result; + data->state.creds_from = CREDS_NETRC; } if(data->state.aptr.passwd) { uc = curl_url_set(data->state.uh, CURLUPART_PASSWORD, diff --git a/lib/urldata.h b/lib/urldata.h index ce28f25..b68d023 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -1207,6 +1207,11 @@ struct urlpieces { char *query; }; +#define CREDS_NONE 0 +#define CREDS_URL 1 /* from URL */ +#define CREDS_OPTION 2 /* set with a CURLOPT_ */ +#define CREDS_NETRC 3 /* found in netrc */ + struct UrlState { /* Points to the connection cache */ struct conncache *conn_cache; @@ -1344,7 +1349,6 @@ struct UrlState { char *proxyuser; char *proxypasswd; } aptr; - unsigned char httpwant; /* when non-zero, a specific HTTP version requested to be used in the library's request(s) */ unsigned char httpversion; /* the lowest HTTP version*10 reported by any @@ -1354,6 +1358,9 @@ struct UrlState { unsigned char select_bits; /* != 0 -> bitmask of socket events for this transfer overriding anything the socket may report */ + unsigned int creds_from:2; /* where is the server credentials originating + from, see the CREDS_* defines above */ + #ifdef CURLDEBUG BIT(conncache_lock); #endif diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc index d89e565..03cb6a0 100644 --- a/tests/data/Makefile.inc +++ b/tests/data/Makefile.inc @@ -126,7 +126,7 @@ test952 test953 test954 test955 test956 test957 test958 test959 test960 \ test961 test962 test963 test964 test965 test966 test967 test968 test969 \ test970 test971 test972 test973 test974 test975 test976 test977 test978 \ test979 test980 test981 test982 test983 test984 test985 test986 test987 \ -test988 test989 test990 test991 test992 \ +test988 test989 test990 test991 test992 test998 test999 \ \ test1000 test1001 test1002 test1003 test1004 test1005 test1006 test1007 \ test1008 test1009 test1010 test1011 test1012 test1013 test1014 test1015 \ diff --git a/tests/data/test998 b/tests/data/test998 new file mode 100644 index 0000000..596b18e --- /dev/null +++ b/tests/data/test998 @@ -0,0 +1,92 @@ + + + + HTTP + --location-trusted + + + + # + # Server-side + + + HTTP/1.1 301 redirect + Date: Tue, 09 Nov 2010 14:49:00 GMT + Server: test-server/fake + Content-Length: 0 + Connection: close + Content-Type: text/html + Location: http://somewhere.else.example/a/path/%TESTNUMBER0002 + + + + HTTP/1.1 200 OK + Date: Tue, 09 Nov 2010 14:49:00 GMT + Content-Length: 6 + Content-Type: text/html + Funny-head: yesyes + + -foo- + + + + HTTP/1.1 301 redirect + Date: Tue, 09 Nov 2010 14:49:00 GMT + Server: test-server/fake + Content-Length: 0 + Connection: close + Content-Type: text/html + Location: http://somewhere.else.example/a/path/%TESTNUMBER0002 + + HTTP/1.1 200 OK + Date: Tue, 09 Nov 2010 14:49:00 GMT + Content-Length: 6 + Content-Type: text/html + Funny-head: yesyes + + -foo- + + + + + # + # Client-side + + + proxy + + + http + + + HTTP with auth in URL redirected to another host + + + -x %HOSTIP:%HTTPPORT http://alberto:einstein@somwhere.example/%TESTNUMBER --location-trusted + + + + # + # Verify data after the test has been "shot" + + + QUIT + + + GET http://somwhere.example/998 HTTP/1.1 + Host: somwhere.example + Authorization: Basic YWxiZXJ0bzplaW5zdGVpbg== + User-Agent: curl/%VERSION + Accept: */* + Proxy-Connection: Keep-Alive + + GET http://somewhere.else.example/a/path/9980002 HTTP/1.1 + Host: somewhere.else.example + Authorization: Basic YWxiZXJ0bzplaW5zdGVpbg== + User-Agent: curl/%VERSION + Accept: */* + Proxy-Connection: Keep-Alive + + + + diff --git a/tests/data/test999 b/tests/data/test999 new file mode 100644 index 0000000..184821d --- /dev/null +++ b/tests/data/test999 @@ -0,0 +1,81 @@ + + + + HTTP + --location-trusted + + + + # + # Server-side + + + HTTP/1.1 200 OK + Date: Tue, 09 Nov 2010 14:49:00 GMT + Content-Length: 6 + Content-Type: text/html + Funny-head: yesyes + + -foo- + + + + HTTP/1.1 301 redirect + Date: Tue, 09 Nov 2010 14:49:00 GMT + Server: test-server/fake + Content-Length: 0 + Connection: close + Content-Type: text/html + Location: http://somewhere.else.example/a/path/%TESTNUMBER0002 + + HTTP/1.1 200 OK + Date: Tue, 09 Nov 2010 14:49:00 GMT + Content-Length: 6 + Content-Type: text/html + Funny-head: yesyes + + -foo- + + + + + # + # Client-side + + + proxy + + + http + + + HTTP with auth in first URL but not second + + + -x %HOSTIP:%HTTPPORT http://alberto:einstein@somwhere.example/%TESTNUMBER http://somewhere.else.example/%TESTNUMBER + + + + # + # Verify data after the test has been "shot" + + + QUIT + + + GET http://somwhere.example/%TESTNUMBER HTTP/1.1 + Host: somwhere.example + Authorization: Basic YWxiZXJ0bzplaW5zdGVpbg== + User-Agent: curl/%VERSION + Accept: */* + Proxy-Connection: Keep-Alive + + GET http://somewhere.else.example/%TESTNUMBER HTTP/1.1 + Host: somewhere.else.example + User-Agent: curl/%VERSION + Accept: */* + Proxy-Connection: Keep-Alive + + + + -- 2.40.0