From: Alhuda Khan Date: Sun, 12 Jul 2026 12:01:44 +0000 (+0530) Subject: ftp: reject control bytes in ACCT and alternative-to-user X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=94947509868bab0219945567632b519416b1dfb4;p=thirdparty%2Fcurl.git ftp: reject control bytes in ACCT and alternative-to-user A CR or LF in the CURLOPT_FTP_ACCOUNT or CURLOPT_FTP_ALTERNATIVE_TO_USER string split the control-channel command line and smuggled a second FTP command. Reject a byte below 0x20 in both values before the command is built. Closes #22301 --- diff --git a/lib/ftp.c b/lib/ftp.c index b3da888b44..f3c07d6722 100644 --- a/lib/ftp.c +++ b/lib/ftp.c @@ -2927,6 +2927,20 @@ static CURLcode ftp_state_loggedin(struct Curl_easy *data, return result; } +/* A value that becomes part of an FTP control command must not carry a + control byte: a CR or LF would end the command line and let a second + command be smuggled onto the control connection. */ +static bool ftp_has_ctrl(const char *string) +{ + const unsigned char *s = (const unsigned char *)string; + while(*s) { + if(*s < 0x20) + return TRUE; + s++; + } + return FALSE; +} + /* for USER and PASS responses */ static CURLcode ftp_state_user_resp(struct Curl_easy *data, struct ftp_conn *ftpc, @@ -2949,16 +2963,20 @@ static CURLcode ftp_state_user_resp(struct Curl_easy *data, result = ftp_state_loggedin(data, ftpc); } else if(ftpcode == 332) { - if(data->set.str[STRING_FTP_ACCOUNT]) { - result = Curl_pp_sendf(data, &ftpc->pp, "ACCT %s", - data->set.str[STRING_FTP_ACCOUNT]); - if(!result) - ftp_state(data, ftpc, FTP_ACCT); - } - else { + const char *account = data->set.str[STRING_FTP_ACCOUNT]; + if(!account) { failf(data, "ACCT requested but none available"); result = CURLE_LOGIN_DENIED; } + else if(ftp_has_ctrl(account)) { + failf(data, "Control byte in FTP account"); + result = CURLE_BAD_FUNCTION_ARGUMENT; + } + else { + result = Curl_pp_sendf(data, &ftpc->pp, "ACCT %s", account); + if(!result) + ftp_state(data, ftpc, FTP_ACCT); + } } else { /* All other response codes, like: @@ -2966,14 +2984,19 @@ static CURLcode ftp_state_user_resp(struct Curl_easy *data, 530 User ... access denied (the server denies to log the specified user) */ - if(data->set.str[STRING_FTP_ALTERNATIVE_TO_USER] && - !ftpc->ftp_trying_alternative) { + const char *alt = data->set.str[STRING_FTP_ALTERNATIVE_TO_USER]; + if(alt && !ftpc->ftp_trying_alternative) { /* Ok, USER failed. Let's try the supplied command. */ - result = Curl_pp_sendf(data, &ftpc->pp, "%s", - data->set.str[STRING_FTP_ALTERNATIVE_TO_USER]); - if(!result) { - ftpc->ftp_trying_alternative = TRUE; - ftp_state(data, ftpc, FTP_USER); + if(ftp_has_ctrl(alt)) { + failf(data, "Control byte in FTP alternative-to-user command"); + result = CURLE_BAD_FUNCTION_ARGUMENT; + } + else { + result = Curl_pp_sendf(data, &ftpc->pp, "%s", alt); + if(!result) { + ftpc->ftp_trying_alternative = TRUE; + ftp_state(data, ftpc, FTP_USER); + } } } else { diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index e124958874..45df544649 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -254,7 +254,7 @@ test2072 test2073 test2074 test2075 test2076 test2077 test2078 test2079 \ test2080 test2081 test2082 test2083 test2084 test2085 test2086 test2087 \ test2088 test2089 test2090 test2091 test2092 \ test2100 test2101 test2102 test2103 test2104 test2105 test2106 test2107 \ -test2108 test2109 test2110 test2113 test2114 \ +test2108 test2109 test2110 test2113 test2114 test2115 test2116 \ \ test2200 test2201 test2202 test2203 test2204 test2205 test2206 test2207 \ test2208 \ diff --git a/tests/data/test2115 b/tests/data/test2115 new file mode 100644 index 0000000000..d9c8b05137 --- /dev/null +++ b/tests/data/test2115 @@ -0,0 +1,46 @@ + + + + +FTP +ACCT + + +# Server-side + + +REPLY PASS 332 please provide account name + + + +# Client-side + + +ftp + + +FTP --ftp-account with embedded CRLF is rejected + +# read the account from a config file so the raw CR LF reaches curl intact + +ftp-account = "one\r\nDELE %TESTNUMBER" + + +ftp://%HOSTIP:%FTPPORT/%TESTNUMBER -K %LOGDIR/test%TESTNUMBER.config + + + +# Verify data after the test has been "shot" + +# 43 - CURLE_BAD_FUNCTION_ARGUMENT + +43 + +# the account is rejected before ACCT is built, so the injected DELE command +# must never reach the server + +USER anonymous +PASS ftp@example.com + + + diff --git a/tests/data/test2116 b/tests/data/test2116 new file mode 100644 index 0000000000..8ecd679b4a --- /dev/null +++ b/tests/data/test2116 @@ -0,0 +1,45 @@ + + + + +FTP +--ftp-alternative-to-user + + +# Server-side + + +REPLY USER 530 go away + + + +# Client-side + + +ftp + + +FTP --ftp-alternative-to-user with embedded CRLF is rejected + +# read the command from a config file so the raw CR LF reaches curl intact + +ftp-alternative-to-user = "USER me\r\nDELE %TESTNUMBER" + + +ftp://%HOSTIP:%FTPPORT/%TESTNUMBER/ -K %LOGDIR/test%TESTNUMBER.config + + + +# Verify data after the test has been "shot" + +# 43 - CURLE_BAD_FUNCTION_ARGUMENT + +43 + +# the replacement command is rejected before it is sent, so the injected DELE +# command must never reach the server + +USER anonymous + + +