ftp: Limit pending commands to FTP::max_pending_commands (default 20)

scripts/base/frameworks/notice/weird.zeek

@@ -136,6 +136,7 @@ export {
 		["FIN_advanced_last_seq"]               = ACTION_LOG,
 		["FIN_after_reset"]                     = ACTION_IGNORE,
 		["FIN_storm"]                           = ACTION_NOTICE_PER_ORIG,
+		["FTP_too_many_pending_commands"]       = ACTION_LOG_PER_CONN,
 		["HTTP_bad_chunk_size"]                 = ACTION_LOG,
 		["HTTP_chunked_transfer_for_multipart_message"] = ACTION_LOG,
 		["HTTP_overlapping_messages"]           = ACTION_LOG,

scripts/base/protocols/ftp/main.zeek

@@ -10,6 +10,7 @@
 @load base/utils/numbers
 @load base/utils/addrs
 @load base/frameworks/cluster
+@load base/frameworks/notice/weird
 @load base/protocols/conn/removal-hooks
 
 module FTP;
@@ -51,6 +52,11 @@ export {
 	## FTP data finalization hook.  Expected FTP data channel state may
 	## get purged when called.
 	global finalize_ftp_data: hook(c: connection);
+
+	## Allow a client to send this many commands before the server
+	## sends a reply. If this value is exceeded a weird named
+	## FTP_too_many_pending_commands is logged for the connection.
+	option max_pending_commands = 20;
 }
 
 # Add the state tracking information variable to the connection record
@@ -229,7 +235,11 @@ event ftp_request(c: connection, command: string, arg: string) &priority=5
 	set_ftp_session(c);
 
 	# Queue up the new command and argument
-	add_pending_cmd(c$ftp$pending_commands, command, arg);
+	if ( |c$ftp$pending_commands| < max_pending_commands )
+		add_pending_cmd(c$ftp$pending_commands, command, arg);
+	else
+		Reporter::conn_weird("FTP_too_many_pending_commands", c,
+				     cat(|c$ftp$pending_commands|), "FTP");
 
 	if ( command == "USER" )
 		c$ftp$user = arg;

testing/btest/Baseline/plugins.hooks/output

@@ -505,6 +505,7 @@
 0.000000   MetaHookPost  CallFunction(Option::set_change_handler, <frame>, (FTP::default_capture_password, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)) -> <no result>
 0.000000   MetaHookPost  CallFunction(Option::set_change_handler, <frame>, (FTP::guest_ids, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)) -> <no result>
 0.000000   MetaHookPost  CallFunction(Option::set_change_handler, <frame>, (FTP::logged_commands, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)) -> <no result>
+0.000000   MetaHookPost  CallFunction(Option::set_change_handler, <frame>, (FTP::max_pending_commands, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)) -> <no result>
 0.000000   MetaHookPost  CallFunction(Option::set_change_handler, <frame>, (FileExtract::default_limit, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)) -> <no result>
 0.000000   MetaHookPost  CallFunction(Option::set_change_handler, <frame>, (Files::enable_reassembler, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)) -> <no result>
 0.000000   MetaHookPost  CallFunction(Option::set_change_handler, <frame>, (GridFTP::max_time, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)) -> <no result>
@@ -2020,6 +2021,7 @@
 0.000000   MetaHookPre   CallFunction(Option::set_change_handler, <frame>, (FTP::default_capture_password, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100))
 0.000000   MetaHookPre   CallFunction(Option::set_change_handler, <frame>, (FTP::guest_ids, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100))
 0.000000   MetaHookPre   CallFunction(Option::set_change_handler, <frame>, (FTP::logged_commands, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100))
+0.000000   MetaHookPre   CallFunction(Option::set_change_handler, <frame>, (FTP::max_pending_commands, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100))
 0.000000   MetaHookPre   CallFunction(Option::set_change_handler, <frame>, (FileExtract::default_limit, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100))
 0.000000   MetaHookPre   CallFunction(Option::set_change_handler, <frame>, (Files::enable_reassembler, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100))
 0.000000   MetaHookPre   CallFunction(Option::set_change_handler, <frame>, (GridFTP::max_time, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100))
@@ -3534,6 +3536,7 @@
 0.000000 | HookCallFunction Option::set_change_handler(FTP::default_capture_password, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)
 0.000000 | HookCallFunction Option::set_change_handler(FTP::guest_ids, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)
 0.000000 | HookCallFunction Option::set_change_handler(FTP::logged_commands, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)
+0.000000 | HookCallFunction Option::set_change_handler(FTP::max_pending_commands, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)
 0.000000 | HookCallFunction Option::set_change_handler(FileExtract::default_limit, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)
 0.000000 | HookCallFunction Option::set_change_handler(Files::enable_reassembler, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)
 0.000000 | HookCallFunction Option::set_change_handler(GridFTP::max_time, Config::config_option_changed{ Config::log = Config::Info($ts=network_time(), $id=Config::ID, $old_value=Config::format_value(lookup_ID(Config::ID)), $new_value=Config::format_value(Config::new_value))if ( != Config::location) Config::log$location = Config::locationLog::write(Config::LOG, to_any_coerceConfig::log)return (Config::new_value)}, -100)

testing/btest/Baseline/scripts.base.protocols.ftp.ftp-max-pending-commands/conn.log

@@ -0,0 +1,11 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	conn
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	proto	service	duration	orig_bytes	resp_bytes	conn_state	local_orig	local_resp	missed_bytes	history	orig_pkts	orig_ip_bytes	resp_pkts	resp_ip_bytes	tunnel_parents
+#types	time	string	addr	port	addr	port	enum	string	interval	count	count	string	bool	bool	count	string	count	count	count	count	set[string]
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	37950	127.0.0.1	21	tcp	ftp	0.202144	98	261	SF	-	-	0	ShADadfF	21	1198	20	1309	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.ftp.ftp-max-pending-commands/ftp.log

@@ -0,0 +1,12 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	ftp
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	user	password	command	arg	mime_type	file_size	reply_code	reply_msg	data_channel.passive	data_channel.orig_h	data_channel.resp_h	data_channel.resp_p	fuid
+#types	time	string	addr	port	addr	port	string	string	string	string	string	count	count	string	bool	addr	addr	port	string
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	37950	127.0.0.1	21	anonymous	anonymous@zeek.org	USER	anonymous	-	-	230	Fake response USER anonymous	-	-	-	-	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	37950	127.0.0.1	21	anonymous	anonymous@zeek.org	SYST	-	-	-	215	Fake response SYST	-	-	-	-	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.ftp.ftp-max-pending-commands/output

@@ -0,0 +1,18 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+ftp_request, CHhAvVGS1DHFjwGM9, USER, anonymous
+ftp_request, CHhAvVGS1DHFjwGM9, PASS, anonymous@zeek.org
+ftp_request, CHhAvVGS1DHFjwGM9, SYST, 
+ftp_request, CHhAvVGS1DHFjwGM9, TYPE, I
+ftp_request, CHhAvVGS1DHFjwGM9, PASV, 
+ftp_request, CHhAvVGS1DHFjwGM9, RETR, robots.txt
+ftp_request, CHhAvVGS1DHFjwGM9, PORT, 114,115
+ftp_request, CHhAvVGS1DHFjwGM9, QUIT, 
+ftp_reply, CHhAvVGS1DHFjwGM9, 220, py.fake ready
+ftp_reply, CHhAvVGS1DHFjwGM9, 230, Fake response USER anonymous
+ftp_reply, CHhAvVGS1DHFjwGM9, 230, Fake response PASS anonymous@zeek.org
+ftp_reply, CHhAvVGS1DHFjwGM9, 215, Fake response SYST
+ftp_reply, CHhAvVGS1DHFjwGM9, 200, Fake response TYPE I
+ftp_reply, CHhAvVGS1DHFjwGM9, 227, Fake response PASV
+ftp_reply, CHhAvVGS1DHFjwGM9, 225, Fake response RETR robots.txt
+ftp_reply, CHhAvVGS1DHFjwGM9, 200, Fake response PORT 114,115
+ftp_reply, CHhAvVGS1DHFjwGM9, 221, Fake response QUIT

testing/btest/Baseline/scripts.base.protocols.ftp.ftp-max-pending-commands/weird.log

@@ -0,0 +1,11 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	weird
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	name	addl	notice	peer	source
+#types	time	string	addr	port	addr	port	string	string	bool	string	string
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	37950	127.0.0.1	21	FTP_too_many_pending_commands	5	F	zeek	FTP
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/scripts/base/protocols/ftp/ftp-max-pending-commands.zeek

@@ -0,0 +1,24 @@
+# @TEST-DOC: Artificially generated pcap where the FTP client sends a batch of commands before the server ever responds with a ready message. Cap max_pending_commands at 5 and verify generation of weird.log and no scripting errors.
+#
+# @TEST-EXEC: zeek -b -r $TRACES/ftp/fake-server-delays-all.pcap %INPUT > output
+# @TEST-EXEC: btest-diff output
+# @TEST-EXEC: btest-diff conn.log
+# @TEST-EXEC: btest-diff ftp.log
+# @TEST-EXEC: btest-diff weird.log
+# @TEST-EXEC: test ! -f reporter.log
+
+@load base/protocols/conn
+@load base/protocols/ftp
+
+redef FTP::max_pending_commands = 5;
+redef FTP::logged_commands += { "USER", "SYST" };
+
+event ftp_request(c: connection, command: string, arg: string)
+	{
+	print "ftp_request", c$uid, command, arg;
+	}
+
+event ftp_reply(c: connection, code: count, msg: string, cont_resp: bool)
+	{
+	print "ftp_reply", c$uid, code, msg;
+	}