Merge remote-tracking branch 'security/topic/awelzel/148-ftp-skip-get-pending-commands-multi-line-response'

* security/topic/awelzel/148-ftp-skip-get-pending-commands-multi-line-response:
  ftp/main: Special case for intermediate reply lines
  ftp/main: Skip get_pending_command() for intermediate reply lines

CHANGES

@@ -1,3 +1,38 @@
+6.0.0-dev.336 | 2023-04-11 14:50:47 -0700
+
+  * ftp/main: Special case for intermediate reply lines (Arne Welzel, Corelight)
+
+    The medium.trace in the private external test suite contains one
+    session/server that violates the multi-line reply protocol and
+    happened to work out fairly well regardless due to how we looked
+    up the pending commands unconditionally before.
+
+    Continue to match up reply lines that "look like they contain status codes"
+    even if cont_resp = T. This still improves runtime for the OSS-Fuzz
+    generated test case and keeps the external baselines valid.
+
+    The affected session can be extracted as follows:
+
+        zcat Traces/medium.trace.gz | tcpdump -r  - 'port 1491 and port 21'
+
+    We could push this into the analyzer, too, minimally the RFC says:
+
+        > If an intermediary line begins with a 3-digit number, the Server
+        > must pad the front  to avoid confusion.
+
+  * ftp/main: Skip get_pending_command() for intermediate reply lines (Arne Welzel, Corelight)
+
+    Intermediate lines of multiline replies usually do not contain valid status
+    codes (even if servers may opt to include them). Their content may be anything
+    and likely unrelated to the original command. There's little reason for us
+    trying to match them with a corresponding command.
+
+    OSS-Fuzz generated a large command reply with very many intermediate lines
+    which caused long processing times due to matching every line with all
+    currently pending commands.
+    This is a DoS vector against Zeek. The new ipv6-multiline-reply.trace and
+    ipv6-retr-samba.trace files have been extracted from the external ipv6.trace.
+
 6.0.0-dev.333 | 2023-04-11 12:05:29 -0700
 
   * Add cstdint to WeirdState.h to fix compilation error on gcc13 (Tim Wojtulewicz, Corelight)

VERSION

@@ -1 +1 @@
-6.0.0-dev.333
+6.0.0-dev.336

scripts/base/protocols/ftp/main.zeek

@@ -316,12 +316,58 @@ event ftp_request(c: connection, command: string, arg: string) &priority=5
 event ftp_reply(c: connection, code: count, msg: string, cont_resp: bool) &priority=5
 	{
 	set_ftp_session(c);
+
+	# Skip matching up intermediate reply lines (that do not have a
+	# valid status code) with pending commands. Because they may not
+	# have a proper status code, there's little point setting whatever
+	# their reply_code and reply_msg are on the command.
+	#
+	# There's a quirk: Some FTP servers return(ed?) replies like the
+	# following, violating the multi-line reply protocol:
+	#
+	#  c: STOR intermol.ps
+	#  s: 150 Opening ASCII mode data connection for 'intermol.ps'.
+	#  s: 230- WARNING! 4 bare linefeeds received in ASCII mode
+	#  s:    File may not have transferred correctly.
+	#  s: 226 Transfer complete.
+	#
+	# This is a multiline response started with 230-, but never finalized
+	# with the same status code. It should have been completed with
+	# "230 <some final message>", but instead was completed with "226 ...".
+	# This confuses our parser, returning cont_resp = T for all following
+	# server messages. This caused a regression as the current command wasn't
+	# updated for logging.
+	#
+	# The regex below is a best effort to keep existing behavior
+	# in face of such traffic. It matches on messages that look
+	# like valid status codes (starting with 3 digits followed by
+	# at least 10 ASCII characters).
+	#
+	# There's the following in RFC 959, so in the future we could push
+	# the detection/logic down into the parser instead of here.
+	#
+	#   If an intermediary line begins with a 3-digit number, the Server
+	#   must pad the front to avoid confusion.
+	#
+	if ( cont_resp && code == 0 && c$ftp?$reply_code )
+		{
+		if ( /^[1-9][0-9]{2} [[:print:]]{10}.*/ !in msg )
+			return;
+		else
+			{
+			# This might be worth a weird, but not sure it's
+			# worth it and how trigger happy it could be.
+			# Reporter::conn_weird("FTP_intermediate_line_with_reply_code", c, msg, "FTP");
+			}
+		}
+
 	c$ftp$cmdarg = get_pending_cmd(c$ftp$pending_commands, code, msg);
 	c$ftp$reply_code = code;
 	c$ftp$reply_msg = msg;
 
-	# TODO: figure out what to do with continued FTP response (not used much)
-	if ( cont_resp ) return;
+	# Do not parse out information from any but the first reply line.
+	if ( cont_resp )
+		return;
 
 	# TODO: do some sort of generic clear text login processing here.
 	local response_xyz = parse_ftp_reply_code(code);

testing/btest/Baseline/scripts.base.protocols.ftp.ftp-multiline-reply/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	2001:470:1f05:17a6:d69a:20ff:fefd:6b88	58895	2400:3000:20:100::46	21	<unknown>	-	<init>	-	-	-	220	FTP server ready.	-	-	-	-	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	2001:470:1f05:17a6:d69a:20ff:fefd:6b88	58895	2400:3000:20:100::46	21	anonymous	-	USER	anonymous	-	-	331	Guest login ok, send your email address as password.	-	-	-	-	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	2001:470:1f05:17a6:d69a:20ff:fefd:6b88	58895	2400:3000:20:100::46	21	anonymous	root@sponge.es.net	PASS	root@sponge.es.net	-	-	230	Guest login ok, access restrictions apply.	-	-	-	-	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	2001:470:1f05:17a6:d69a:20ff:fefd:6b88	58895	2400:3000:20:100::46	21	anonymous	root@sponge.es.net	EPSV	-	-	-	229	Entering Extended Passive Mode (|||60931|)	T	2001:470:1f05:17a6:d69a:20ff:fefd:6b88	2400:3000:20:100::46	60931	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	2001:470:1f05:17a6:d69a:20ff:fefd:6b88	58895	2400:3000:20:100::46	21	anonymous	root@sponge.es.net	RETR	ftp://[2400:3000:20:100::46]/pub/FreeBSD/ports/local-distfiles/avl/libssh-0.5.2.tar.gz	-	-	221	You could at least say goodbye.	-	-	-	-	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.ftp.ftp-multiline-reply/out

@@ -0,0 +1,25 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+ftp_reply, T, 220, Welcome to FTP.JPIX.ad.jp,
+ftp_reply, T, 0, Welcome to FTP.JPIX.ad.jp,
+ftp_reply, T, 0, Welcome to FTP.JPIX.ad.jp,
+ftp_reply, T, 0, Welcome to FTP.JPIX.ad.jp,
+ftp_reply, T, 0, Welcome to FTP.JPIX.ad.jp,
+ftp_reply, T, 0, Welcome to FTP.JPIX.ad.jp,
+ftp_reply, T, 0, Welcome to FTP.JPIX.ad.jp,
+ftp_reply, T, 0, Welcome to FTP.JPIX.ad.jp,
+ftp_reply, T, 0, Welcome to FTP.JPIX.ad.jp,
+ftp_reply, T, 0, Welcome to FTP.JPIX.ad.jp,
+ftp_reply, T, 0, Welcome to FTP.JPIX.ad.jp,
+ftp_reply, F, 220, FTP server ready.
+ftp_reply, F, 331, Guest login ok, send your email address as password.
+ftp_reply, F, 230, Guest login ok, access restrictions apply.
+ftp_reply, F, 257, "/" is current directory.
+ftp_reply, F, 250, CWD command successful.
+ftp_reply, F, 200, MODE S accepted.
+ftp_reply, F, 200, Type set to I.
+ftp_reply, F, 550, libssh-0.5.2.tar.gz: No such file or directory.
+ftp_reply, F, 200, MODE S accepted.
+ftp_reply, F, 200, Type set to I.
+ftp_reply, F, 229, Entering Extended Passive Mode (|||60931|)
+ftp_reply, F, 550, libssh-0.5.2.tar.gz: No such file or directory.
+ftp_reply, F, 221, You could at least say goodbye.

testing/btest/Baseline/scripts.base.protocols.ftp.ftp-samba-retr/ftp.log

@@ -0,0 +1,14 @@
+### 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	2001:470:1f05:17a6:213:72ff:fe0d:a566	16730	2001:6f8:200:1::5:33	21	anonymous	-	USER	anonymous	-	-	331	Anonymous login ok, send your complete email address as your password	-	-	-	-	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	2001:470:1f05:17a6:213:72ff:fe0d:a566	16730	2001:6f8:200:1::5:33	21	anonymous	root@freebsd-5453	PASS	root@freebsd-5453	-	-	230	Anonymous access granted, restrictions apply	-	-	-	-	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	2001:470:1f05:17a6:213:72ff:fe0d:a566	16730	2001:6f8:200:1::5:33	21	anonymous	root@freebsd-5453	EPSV	-	-	-	229	Entering Extended Passive Mode (|||63282|)	T	2001:470:1f05:17a6:213:72ff:fe0d:a566	2001:6f8:200:1::5:33	63282	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	2001:470:1f05:17a6:213:72ff:fe0d:a566	16730	2001:6f8:200:1::5:33	21	anonymous	root@freebsd-5453	RETR	ftp://[2001:6f8:200:1::5:33]/samba/samba-3.4.17.tar.gz	-	34826629	226	Transfer complete	-	-	-	-	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.ftp.ftp-samba-retr/out

@@ -0,0 +1,16 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+ftp_reply, F, 220, 2001:6f8:200:1::5:33 FTP server ready
+ftp_reply, F, 331, Anonymous login ok, send your complete email address as your password
+ftp_reply, F, 230, Anonymous access granted, restrictions apply
+ftp_reply, F, 257, "/" is the current directory
+ftp_reply, T, 250, See http://samba.org/ for a list of mirror sites
+ftp_reply, F, 250, CWD command successful
+ftp_reply, F, 200, Mode set to S
+ftp_reply, F, 200, Type set to I
+ftp_reply, F, 213, 34826629
+ftp_reply, F, 213, 20120430122210
+ftp_reply, F, 200, Mode set to S
+ftp_reply, F, 200, Type set to I
+ftp_reply, F, 229, Entering Extended Passive Mode (|||63282|)
+ftp_reply, F, 150, Opening BINARY mode data connection for samba-3.4.17.tar.gz (34826629 bytes)
+ftp_reply, F, 226, Transfer complete

testing/btest/Baseline/scripts.policy.protocols.ftp.ftp/.stderr

@@ -0,0 +1 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.

testing/btest/Baseline/scripts.policy.protocols.ftp.ftp/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	2001:470:1f05:17a6:d69a:20ff:fefd:6b88	58895	2400:3000:20:100::46	21	anonymous	root@sponge.es.net	EPSV	-	-	-	229	Entering Extended Passive Mode (|||60931|)	T	2001:470:1f05:17a6:d69a:20ff:fefd:6b88	2400:3000:20:100::46	60931	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	2001:470:1f05:17a6:d69a:20ff:fefd:6b88	58895	2400:3000:20:100::46	21	anonymous	root@sponge.es.net	RETR	ftp://[2400:3000:20:100::46]/pub/FreeBSD/ports/local-distfiles/avl/libssh-0.5.2.tar.gz	-	-	221	You could at least say goodbye.	-	-	-	-	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/scripts/base/protocols/ftp/ftp-multiline-reply.zeek

@@ -0,0 +1,13 @@
+# @TEST-DOC: Tests that c$ftp$reply_msg stays the same over a multiline reply.
+# @TEST-EXEC: zeek -b -r $TRACES/ftp/ipv6-multiline-reply.trace %INPUT > out
+# @TEST-EXEC: btest-diff ftp.log
+# @TEST-EXEC: btest-diff out
+
+@load base/protocols/conn
+@load base/protocols/ftp
+
+redef FTP::logged_commands += { "<init>", "USER", "PASS" };
+
+event ftp_reply(c: connection, code: count, msg: string, cont_resp: bool) {
+	print "ftp_reply", cont_resp, code, cat(c$ftp$reply_msg);
+}

testing/btest/scripts/base/protocols/ftp/ftp-samba-retr.zeek

@@ -0,0 +1,13 @@
+# @TEST-DOC: Tests interemediate lines to not confuse cwd tracking.
+# @TEST-EXEC: zeek -b -r $TRACES/ftp/ipv6-retr-samba.trace %INPUT > out
+# @TEST-EXEC: btest-diff ftp.log
+# @TEST-EXEC: btest-diff out
+
+@load base/protocols/conn
+@load base/protocols/ftp
+
+redef FTP::logged_commands += { "USER", "PASS", "RETR" };
+
+event ftp_reply(c: connection, code: count, msg: string, cont_resp: bool) {
+	print "ftp_reply", cont_resp, code, cat(c$ftp$reply_msg);
+}

testing/btest/scripts/policy/protocols/ftp/ftp.zeek

@@ -0,0 +1,7 @@
+# @TEST-DOC: Smoke the policy/protocols/ftp scripts don't fall apart.
+# @TEST-EXEC: zeek -b -r $TRACES/ftp/ipv6-multiline-reply.trace %INPUT
+# @TEST-EXEC: btest-diff ftp.log
+# @TEST-EXEC: btest-diff .stderr
+
+@load protocols/ftp/detect
+@load protocols/ftp/detect-bruteforcing