← Commit list

9494750986

Alhuda Khan

2026-07-13

SUCCESS Duration: 194s

Subject

ftp: reject control bytes in ACCT and alternative-to-user

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

lib/ftp.c +37 -14 tests/data/Makefile.am +1 -1 tests/data/test2115 +46 -0 tests/data/test2116 +45 -0

lib/ftp.c+37 -14

@@ -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 {

tests/data/Makefile.am+1 -1

@@ -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 \

tests/data/test2115+46 -0

@@ -0,0 +1,46 @@
+	<?xml version="1.0" encoding="US-ASCII"?>
+	<testcase>
+	<info>
+	<keywords>
+	FTP
+	ACCT
+	</keywords>
+	</info>
+	# Server-side
+	<reply>
+	<servercmd>
+	REPLY PASS 332 please provide account name
+	</servercmd>
+	</reply>
+
+	# Client-side
+	<client>
+	<server>
+	ftp
+	</server>
+	<name>
+	FTP --ftp-account with embedded CRLF is rejected
+	</name>
+	# read the account from a config file so the raw CR LF reaches curl intact
+	<file name="%LOGDIR/test%TESTNUMBER.config">
+	ftp-account = "one\r\nDELE %TESTNUMBER"
+	</file>
+	<command>
+	ftp://%HOSTIP:%FTPPORT/%TESTNUMBER -K %LOGDIR/test%TESTNUMBER.config
+	</command>
+	</client>
+
+	# Verify data after the test has been "shot"
+	<verify>
+	# 43 - CURLE_BAD_FUNCTION_ARGUMENT
+	<errorcode>
+	43
+	</errorcode>
+	# the account is rejected before ACCT is built, so the injected DELE command
+	# must never reach the server
+	<protocol crlf="yes">
+	USER anonymous
+	PASS [email protected]
+	</protocol>
+	</verify>
+	</testcase>

tests/data/test2116+45 -0

@@ -0,0 +1,45 @@
+	<?xml version="1.0" encoding="US-ASCII"?>
+	<testcase>
+	<info>
+	<keywords>
+	FTP
+	--ftp-alternative-to-user
+	</keywords>
+	</info>
+	# Server-side
+	<reply>
+	<servercmd>
+	REPLY USER 530 go away
+	</servercmd>
+	</reply>
+
+	# Client-side
+	<client>
+	<server>
+	ftp
+	</server>
+	<name>
+	FTP --ftp-alternative-to-user with embedded CRLF is rejected
+	</name>
+	# read the command from a config file so the raw CR LF reaches curl intact
+	<file name="%LOGDIR/test%TESTNUMBER.config">
+	ftp-alternative-to-user = "USER me\r\nDELE %TESTNUMBER"
+	</file>
+	<command>
+	ftp://%HOSTIP:%FTPPORT/%TESTNUMBER/ -K %LOGDIR/test%TESTNUMBER.config
+	</command>
+	</client>
+
+	# Verify data after the test has been "shot"
+	<verify>
+	# 43 - CURLE_BAD_FUNCTION_ARGUMENT
+	<errorcode>
+	43
+	</errorcode>
+	# the replacement command is rejected before it is sent, so the injected DELE
+	# command must never reach the server
+	<protocol crlf="yes">
+	USER anonymous
+	</protocol>
+	</verify>
+	</testcase>