Merge remote-tracking branch 'security/topic/awelzel/121-ftp-timeout-again'

* security/topic/awelzel/121-ftp-timeout-again:
  ftp: Introduce FTP::max_command_length

CHANGES

@@ -1,3 +1,15 @@
+5.2.0-dev.357 | 2022-11-22 12:27:26 -0700
+
+  * ftp: Introduce FTP::max_command_length (Arne Welzel, Corelight)
+
+    oss-fuzz produced FTP traffic with a ~550KB long FTP command. Cap FTP command
+    length at 100 bytes, log a weird if a command is larger than that and move
+    on to the next. Likely it's not actual FTP traffic, but raising an
+    analyzer violation would allow clients an easy way to disable the analyzer
+    by sending an overly long command.
+
+    The added test PCAP was generated using a fake Python socket server/client.
+
 5.2.0-dev.355 | 2022-11-22 12:26:50 -0700
 
   * http: Heuristic around rejecting malformed HTTP/0.9 traffic (Arne Welzel, Corelight)

VERSION

@@ -1 +1 @@
-5.2.0-dev.355
+5.2.0-dev.357

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_max_command_length_exceeded"]     = ACTION_LOG_PER_CONN,
 		["FTP_too_many_pending_commands"]       = ACTION_LOG_PER_CONN,
 		["HTTP_bad_chunk_size"]                 = ACTION_LOG,
 		["HTTP_chunked_transfer_for_multipart_message"] = ACTION_LOG,

scripts/base/init-bare.zeek

@@ -337,6 +337,15 @@ type ftp_port: record {
 	valid: bool;	##< True if format was right. Only then are *h* and *p* valid.
 };
 
+
+module FTP;
+
+## Limits the size of commands accepted by the FTP analyzer. Longer commands
+## raise a FTP_max_command_length_exceeded weird and are discarded.
+const max_command_length = 100 &redef;
+
+module GLOBAL;
+
 ## Statistics about what a TCP endpoint sent.
 ##
 ## .. zeek:see:: conn_stats

src/analyzer/protocol/ftp/FTP.cc

@@ -96,6 +96,17 @@ void FTP_Analyzer::DeliverStream(int length, const u_char* data, bool orig)
 			// Weird("FTP command missing", end_of_line - orig_line, orig_line);
 			cmd_str = new StringVal("<missing>");
 			}
+		else if ( BifConst::FTP::max_command_length > 0 &&
+		          static_cast<zeek_uint_t>(cmd_len) > BifConst::FTP::max_command_length )
+			{
+			// If the FTP command is unusually long, log a weird if the analyzer
+			// has previously been confirmed, but otherwise just ignore the whole
+			// line and move on to the next.
+			if ( AnalyzerConfirmed() )
+				Weird("FTP_max_command_length_exceeded", util::fmt("%d", cmd_len));
+
+			return;
+			}
 		else
 			cmd_str = (new StringVal(cmd_len, cmd))->ToUpper();
 

src/const.bif

@@ -11,6 +11,8 @@ const exit_only_after_terminate: bool;
 const digest_salt: string;
 const max_analyzer_violations: count;
 
+const FTP::max_command_length: count;
+
 const NFS3::return_data: bool;
 const NFS3::return_data_max: count;
 const NFS3::return_data_first_only: bool;

testing/btest/Baseline/scripts.base.protocols.ftp.ftp-max-command-length/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	58634	127.0.0.1	21	tcp	ftp	0.213412	358	313	SF	-	-	0	ShAdDaFf	23	1562	17	1205	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.ftp.ftp-max-command-length/dpd.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	dpd
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	proto	analyzer	failure_reason
+#types	time	string	addr	port	addr	port	enum	string	string
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	58634	127.0.0.1	21	tcp	FTP	FTP::max_command_length exceeded
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.ftp.ftp-max-command-length/ftp.log

@@ -0,0 +1,15 @@
+### 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	58634	127.0.0.1	21	anonymous	-	USER	anonymous	-	-	230	Response to USER command	-	-	-	-	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	58634	127.0.0.1	21	anonymous	anonymous@zeek.org	SYST	-	-	-	200	Response to TYPEEEEEEEEEEEEE command	-	-	-	-	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	58634	127.0.0.1	21	anonymous	anonymous@zeek.org	PASV	-	-	-	215	Response to SYSTTTTTTTTTTTTT command	-	-	-	-	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	58634	127.0.0.1	21	anonymous	anonymous@zeek.org	RETR	ftp://127.0.0.1/./robots.txt	-	-	225	Response to RETR command	-	-	-	-	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	58634	127.0.0.1	21	anonymous	anonymous@zeek.org	PORT	114,115	-	-	200	Response to PORT command	-	-	-	-	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.ftp.ftp-max-command-length/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	58634	127.0.0.1	21	FTP_max_command_length_exceeded	132	F	zeek	FTP
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/scripts/base/protocols/ftp/ftp-max-command-length.zeek

@@ -0,0 +1,12 @@
+# @TEST-DOC: Artificially generated pcap with FTP commands of length > 100. Verify generation of the involved logs.
+#
+# @TEST-EXEC: zeek -b -r $TRACES/ftp/fake-long-commands.pcap %INPUT
+# @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::logged_commands += { "USER", "SYST" };