From 0fb4e4f5224d132600e0526fea9aa741b0c11651 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Mon, 6 Mar 2023 12:07:33 +0100 Subject: [PATCH 1/6] telnet: only accept option arguments in ascii To avoid embedded telnet negotiation commands etc. Reported-by: Harry Sintonen Closes #10728 Signed-off-by: zhouhaifeng --- lib/telnet.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/lib/telnet.c b/lib/telnet.c index 579320341..8f3c562ef 100644 --- a/lib/telnet.c +++ b/lib/telnet.c @@ -768,6 +768,17 @@ static void printsub(struct Curl_easy *data, } } +static bool str_is_nonascii(const char *str) +{ + size_t len = strlen(str); + while(len--) { + if(*str & 0x80) + return TRUE; + str++; + } + return FALSE; +} + static CURLcode check_telnet_options(struct Curl_easy *data) { struct curl_slist *head; @@ -782,6 +793,8 @@ static CURLcode check_telnet_options(struct Curl_easy *data) /* Add the user name as an environment variable if it was given on the command line */ if(conn->bits.user_passwd) { + if(str_is_nonascii(conn->user)) + return CURLE_BAD_FUNCTION_ARGUMENT; msnprintf(option_arg, sizeof(option_arg), "USER,%s", conn->user); beg = curl_slist_append(tn->telnet_vars, option_arg); if(!beg) { @@ -797,6 +810,9 @@ static CURLcode check_telnet_options(struct Curl_easy *data) if(sscanf(head->data, "%127[^= ]%*[ =]%255s", option_keyword, option_arg) == 2) { + if(str_is_nonascii(option_arg)) + continue; + /* Terminal type */ if(strcasecompare(option_keyword, "TTYPE")) { strncpy(tn->subopt_ttype, option_arg, 31); -- Gitee From 1fee2a06e6dbb5c512b92f49e6f05dae41e98a6d Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Thu, 9 Mar 2023 16:22:11 +0100 Subject: [PATCH 2/6] curl_path: create the new path with dynbuf Closes #10729 Signed-off-by: zhouhaifeng --- lib/curl_path.c | 71 ++++++++++++++++++++++++------------------------- 1 file changed, 35 insertions(+), 36 deletions(-) diff --git a/lib/curl_path.c b/lib/curl_path.c index 65106188c..5dc6966e1 100644 --- a/lib/curl_path.c +++ b/lib/curl_path.c @@ -30,66 +30,65 @@ #include "escape.h" #include "memdebug.h" +#define MAX_SSHPATH_LEN 100000 /* arbitrary */ + /* figure out the path to work with in this particular request */ CURLcode Curl_getworkingpath(struct Curl_easy *data, char *homedir, /* when SFTP is used */ char **path) /* returns the allocated real path to work with */ { - char *real_path = NULL; char *working_path; size_t working_path_len; + struct dynbuf npath; CURLcode result = Curl_urldecode(data, data->state.up.path, 0, &working_path, &working_path_len, REJECT_ZERO); if(result) return result; + /* new path to switch to in case we need to */ + Curl_dyn_init(&npath, MAX_SSHPATH_LEN); + /* Check for /~/, indicating relative to the user's home directory */ - if(data->conn->handler->protocol & CURLPROTO_SCP) { - real_path = malloc(working_path_len + 1); - if(!real_path) { + if((data->conn->handler->protocol & CURLPROTO_SCP) && + (working_path_len > 3) && (!memcmp(working_path, "/~/", 3))) { + /* It is referenced to the home directory, so strip the leading '/~/' */ + if(Curl_dyn_addn(&npath, &working_path[3], working_path_len - 3)) { free(working_path); return CURLE_OUT_OF_MEMORY; } - if((working_path_len > 3) && (!memcmp(working_path, "/~/", 3))) - /* It is referenced to the home directory, so strip the leading '/~/' */ - memcpy(real_path, working_path + 3, working_path_len - 2); - else - memcpy(real_path, working_path, 1 + working_path_len); } - else if(data->conn->handler->protocol & CURLPROTO_SFTP) { - if((working_path_len > 1) && (working_path[1] == '~')) { - size_t homelen = strlen(homedir); - real_path = malloc(homelen + working_path_len + 1); - if(!real_path) { - free(working_path); - return CURLE_OUT_OF_MEMORY; - } - /* It is referenced to the home directory, so strip the - leading '/' */ - memcpy(real_path, homedir, homelen); - real_path[homelen] = '/'; - real_path[homelen + 1] = '\0'; - if(working_path_len > 3) { - memcpy(real_path + homelen + 1, working_path + 3, - 1 + working_path_len -3); - } + else if((data->conn->handler->protocol & CURLPROTO_SFTP) && + (working_path_len > 2) && !memcmp(working_path, "/~/", 3)) { + size_t len; + const char *p; + int copyfrom = 3; + if(Curl_dyn_add(&npath, homedir)) { + free(working_path); + return CURLE_OUT_OF_MEMORY; } - else { - real_path = malloc(working_path_len + 1); - if(!real_path) { - free(working_path); - return CURLE_OUT_OF_MEMORY; - } - memcpy(real_path, working_path, 1 + working_path_len); + /* Copy a separating '/' if homedir does not end with one */ + len = Curl_dyn_len(&npath); + p = Curl_dyn_ptr(&npath); + if(len && (p[len-1] != '/')) + copyfrom = 2; + + if(Curl_dyn_addn(&npath, + &working_path[copyfrom], working_path_len - copyfrom)) { + free(working_path); + return CURLE_OUT_OF_MEMORY; } } - free(working_path); + if(Curl_dyn_len(&npath)) { + free(working_path); - /* store the pointer for the caller to receive */ - *path = real_path; + /* store the pointer for the caller to receive */ + *path = Curl_dyn_ptr(&npath); + } + else + *path = working_path; return CURLE_OK; } -- Gitee From 1c4bde2116f2670ba161277c576cc574c7d32c9d Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Thu, 6 Oct 2022 00:49:10 +0200 Subject: [PATCH 3/6] 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 Signed-off-by: zhouhaifeng --- 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 0a4ae2cdc..b771b6098 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 cd04f2c0b..c886be1f1 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 127bfdd44..aa47814d6 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 2569f6d11..8bbe942f7 100644 --- a/lib/url.c +++ b/lib/url.c @@ -936,19 +936,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 != !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; } @@ -1333,10 +1324,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; } @@ -1412,8 +1403,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 */ @@ -1435,8 +1426,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 94f8f8c0d..a413419bd 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 29aa01817..ec72d90c8 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) && -- Gitee From 6b13115a147c0be24a41e870db89815a6e028cec Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Thu, 9 Mar 2023 17:47:06 +0100 Subject: [PATCH 4/6] ftp: add more conditions for connection reuse Reported-by: Harry Sintonen Closes #10730 Signed-off-by: zhouhaifeng --- lib/ftp.c | 28 ++++++++++++++++++++++++++-- lib/ftp.h | 5 +++++ lib/setopt.c | 2 +- lib/url.c | 17 +++++++++++++++-- lib/urldata.h | 4 ++-- 5 files changed, 49 insertions(+), 7 deletions(-) diff --git a/lib/ftp.c b/lib/ftp.c index 0b9c9b732..f2f6852e7 100644 --- a/lib/ftp.c +++ b/lib/ftp.c @@ -4094,6 +4094,8 @@ static CURLcode ftp_disconnect(struct Curl_easy *data, } freedirs(ftpc); + Curl_safefree(ftpc->account); + Curl_safefree(ftpc->alternative_to_user); Curl_safefree(ftpc->prevpath); Curl_safefree(ftpc->server_os); Curl_pp_disconnect(pp); @@ -4354,11 +4356,31 @@ static CURLcode ftp_setup_connection(struct Curl_easy *data, { char *type; struct FTP *ftp; + struct ftp_conn *ftpc = &conn->proto.ftpc; - data->req.p.ftp = ftp = calloc(sizeof(struct FTP), 1); + ftp = calloc(sizeof(struct FTP), 1); if(NULL == ftp) return CURLE_OUT_OF_MEMORY; + /* clone connection related data that is FTP specific */ + if(data->set.str[STRING_FTP_ACCOUNT]) { + ftpc->account = strdup(data->set.str[STRING_FTP_ACCOUNT]); + if(!ftpc->account) { + free(ftp); + return CURLE_OUT_OF_MEMORY; + } + } + if(data->set.str[STRING_FTP_ALTERNATIVE_TO_USER]) { + ftpc->alternative_to_user = + strdup(data->set.str[STRING_FTP_ALTERNATIVE_TO_USER]); + if(!ftpc->alternative_to_user) { + Curl_safefree(ftpc->account); + free(ftp); + return CURLE_OUT_OF_MEMORY; + } + } + data->req.p.ftp = ftp; + ftp->path = &data->state.up.path[1]; /* don't include the initial slash */ /* FTP URLs support an extension like ";type=" that @@ -4393,7 +4415,9 @@ static CURLcode ftp_setup_connection(struct Curl_easy *data, /* get some initial data into the ftp struct */ ftp->transfer = PPTRANSFER_BODY; ftp->downloadsize = 0; - conn->proto.ftpc.known_filesize = -1; /* unknown size for now */ + ftpc->known_filesize = -1; /* unknown size for now */ + ftpc->use_ssl = data->set.use_ssl; + ftpc->ccc = data->set.ftp_ccc; return CURLE_OK; } diff --git a/lib/ftp.h b/lib/ftp.h index 1cfdac085..afca25b46 100644 --- a/lib/ftp.h +++ b/lib/ftp.h @@ -115,6 +115,8 @@ struct FTP { struct */ struct ftp_conn { struct pingpong pp; + char *account; + char *alternative_to_user; char *entrypath; /* the PWD reply when we logged on */ char *file; /* url-decoded file name (or path) */ char **dirs; /* realloc()ed array for path components */ @@ -144,6 +146,9 @@ struct ftp_conn { ftpstate state; /* always use ftp.c:state() to change state! */ ftpstate state_saved; /* transfer type saved to be reloaded after data connection is established */ + unsigned char use_ssl; /* if AUTH TLS is to be attempted etc, for FTP or + IMAP or POP3 or others! (type: curl_usessl)*/ + unsigned char ccc; /* ccc level for this connection */ curl_off_t retr_size_saved; /* Size of retrieved file saved */ char *server_os; /* The target server operating system. */ curl_off_t known_filesize; /* file size is different from -1, if wildcard diff --git a/lib/setopt.c b/lib/setopt.c index c6bece9ab..9c2c20bb9 100644 --- a/lib/setopt.c +++ b/lib/setopt.c @@ -2321,7 +2321,7 @@ CURLcode Curl_vsetopt(struct Curl_easy *data, CURLoption option, va_list param) arg = va_arg(param, long); if((arg < CURLUSESSL_NONE) || (arg >= CURLUSESSL_LAST)) return CURLE_BAD_FUNCTION_ARGUMENT; - data->set.use_ssl = (curl_usessl)arg; + data->set.use_ssl = (unsigned char)arg; break; case CURLOPT_SSL_OPTIONS: diff --git a/lib/url.c b/lib/url.c index 8bbe942f7..555df3aa2 100644 --- a/lib/url.c +++ b/lib/url.c @@ -1339,11 +1339,24 @@ ConnectionExists(struct Curl_easy *data, (check->httpversion >= 20) && (data->state.httpwant < CURL_HTTP_VERSION_2_0)) continue; - - if(get_protocol_family(needle->handler) == PROTO_FAMILY_SSH) { +#ifdef USE_SSH + else if(get_protocol_family(needle->handler) & PROTO_FAMILY_SSH) { if(!ssh_config_matches(needle, check)) continue; } +#endif +#ifndef CURL_DISABLE_FTP + else if(get_protocol_family(needle->handler) & PROTO_FAMILY_FTP) { + /* Also match ACCOUNT, ALTERNATIVE-TO-USER, USE_SSL and CCC options */ + if(Curl_timestrcmp(needle->proto.ftpc.account, + check->proto.ftpc.account) || + Curl_timestrcmp(needle->proto.ftpc.alternative_to_user, + check->proto.ftpc.alternative_to_user) || + (needle->proto.ftpc.use_ssl != check->proto.ftpc.use_ssl) || + (needle->proto.ftpc.ccc != check->proto.ftpc.ccc)) + continue; + } +#endif if((needle->handler->flags&PROTOPT_SSL) #ifndef CURL_DISABLE_PROXY diff --git a/lib/urldata.h b/lib/urldata.h index ba76fc794..cda4469d6 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -1745,8 +1745,6 @@ struct UserDefined { enum CURL_NETRC_OPTION use_netrc; /* defined in include/curl.h */ #endif - curl_usessl use_ssl; /* if AUTH TLS is to be attempted etc, for FTP or - IMAP or POP3 or others! */ long new_file_perms; /* Permissions to use when creating remote files */ long new_directory_perms; /* Permissions to use when creating remote dirs */ long ssh_auth_types; /* allowed SSH auth types */ @@ -1870,6 +1868,8 @@ struct UserDefined { BIT(http09_allowed); /* allow HTTP/0.9 responses */ BIT(mail_rcpt_allowfails); /* allow RCPT TO command to fail for some recipients */ + unsigned char use_ssl; /* if AUTH TLS is to be attempted etc, for FTP or + IMAP or POP3 or others! (type: curl_usessl)*/ }; struct Names { -- Gitee From 4636fad83bd73b9c5e24539b9a09cab31337be7c Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Fri, 10 Mar 2023 09:22:43 +0100 Subject: [PATCH 5/6] url: only reuse connections with same GSS delegation Reported-by: Harry Sintonen Closes #10731 Signed-off-by: zhouhaifeng --- lib/url.c | 6 ++++++ lib/urldata.h | 1 + 2 files changed, 7 insertions(+) diff --git a/lib/url.c b/lib/url.c index 555df3aa2..58dd2b456 100644 --- a/lib/url.c +++ b/lib/url.c @@ -1333,6 +1333,11 @@ ConnectionExists(struct Curl_easy *data, } } + /* GSS delegation differences do not actually affect every connection + and auth method, but this check takes precaution before efficiency */ + if(needle->gssapi_delegation != check->gssapi_delegation) + continue; + /* If multiplexing isn't enabled on the h2 connection and h1 is explicitly requested, handle it: */ if((needle->handler->protocol & PROTO_FAMILY_HTTP) && @@ -1803,6 +1808,7 @@ static struct connectdata *allocate_conn(struct Curl_easy *data) conn->fclosesocket = data->set.fclosesocket; conn->closesocket_client = data->set.closesocket_client; conn->lastused = Curl_now(); /* used now */ + conn->gssapi_delegation = data->set.gssapi_delegation; return conn; error: diff --git a/lib/urldata.h b/lib/urldata.h index cda4469d6..03afd814c 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -1126,6 +1126,7 @@ struct connectdata { int socks5_gssapi_enctype; #endif unsigned short localport; + long gssapi_delegation; /* inherited from set.gssapi_delegation */ }; /* The end of connectdata. */ -- Gitee From 3f2eabdf705a8570c65899c52108bd460b6dc6e3 Mon Sep 17 00:00:00 2001 From: zhouhaifeng Date: Fri, 28 Apr 2023 15:07:17 +0800 Subject: [PATCH 6/6] fix -Werror,-Wunused-function when complie mac sdk Signed-off-by: zhouhaifeng --- BUILD.gn | 3 +++ 1 file changed, 3 insertions(+) diff --git a/BUILD.gn b/BUILD.gn index 17b785691..ef2102903 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -306,6 +306,9 @@ if (defined(ohos_lite)) { "-DBUILDING_LIBCURL", "-c", ] + cflags_c = [ + "-Wno-unused-function", + ] libs = [ # AppKit symbols NSFontWeightXXX may be dlsym'ed. "AppKit.framework", -- Gitee