diff --git a/CHANGES b/CHANGES index 2af5894959..0cf307dac1 100644 --- a/CHANGES +++ b/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) diff --git a/VERSION b/VERSION index 5ef77580d4..4f4666ba6d 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -6.0.0-dev.333 +6.0.0-dev.336 diff --git a/scripts/base/protocols/ftp/main.zeek b/scripts/base/protocols/ftp/main.zeek index d56c276886..aceefa0913 100644 --- a/scripts/base/protocols/ftp/main.zeek +++ b/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 ", 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); diff --git a/testing/btest/Baseline/scripts.base.protocols.ftp.ftp-multiline-reply/ftp.log b/testing/btest/Baseline/scripts.base.protocols.ftp.ftp-multiline-reply/ftp.log new file mode 100644 index 0000000000..3901e065d3 --- /dev/null +++ b/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 - - - - 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 diff --git a/testing/btest/Baseline/scripts.base.protocols.ftp.ftp-multiline-reply/out b/testing/btest/Baseline/scripts.base.protocols.ftp.ftp-multiline-reply/out new file mode 100644 index 0000000000..b7d9c53a55 --- /dev/null +++ b/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. diff --git a/testing/btest/Baseline/scripts.base.protocols.ftp.ftp-samba-retr/ftp.log b/testing/btest/Baseline/scripts.base.protocols.ftp.ftp-samba-retr/ftp.log new file mode 100644 index 0000000000..0171f548f6 --- /dev/null +++ b/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 diff --git a/testing/btest/Baseline/scripts.base.protocols.ftp.ftp-samba-retr/out b/testing/btest/Baseline/scripts.base.protocols.ftp.ftp-samba-retr/out new file mode 100644 index 0000000000..b6f51e7bcc --- /dev/null +++ b/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 diff --git a/testing/btest/Baseline/scripts.policy.protocols.ftp.ftp/.stderr b/testing/btest/Baseline/scripts.policy.protocols.ftp.ftp/.stderr new file mode 100644 index 0000000000..49d861c74c --- /dev/null +++ b/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. diff --git a/testing/btest/Baseline/scripts.policy.protocols.ftp.ftp/ftp.log b/testing/btest/Baseline/scripts.policy.protocols.ftp.ftp/ftp.log new file mode 100644 index 0000000000..9b6ac207c3 --- /dev/null +++ b/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 diff --git a/testing/btest/Traces/ftp/ipv6-multiline-reply.trace b/testing/btest/Traces/ftp/ipv6-multiline-reply.trace new file mode 100644 index 0000000000..a5296f437f Binary files /dev/null and b/testing/btest/Traces/ftp/ipv6-multiline-reply.trace differ diff --git a/testing/btest/Traces/ftp/ipv6-retr-samba.trace b/testing/btest/Traces/ftp/ipv6-retr-samba.trace new file mode 100644 index 0000000000..6d4a1334b6 Binary files /dev/null and b/testing/btest/Traces/ftp/ipv6-retr-samba.trace differ diff --git a/testing/btest/scripts/base/protocols/ftp/ftp-multiline-reply.zeek b/testing/btest/scripts/base/protocols/ftp/ftp-multiline-reply.zeek new file mode 100644 index 0000000000..dad917a587 --- /dev/null +++ b/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 += { "", "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); +} diff --git a/testing/btest/scripts/base/protocols/ftp/ftp-samba-retr.zeek b/testing/btest/scripts/base/protocols/ftp/ftp-samba-retr.zeek new file mode 100644 index 0000000000..1ad8c6863a --- /dev/null +++ b/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); +} diff --git a/testing/btest/scripts/policy/protocols/ftp/ftp.zeek b/testing/btest/scripts/policy/protocols/ftp/ftp.zeek new file mode 100644 index 0000000000..bfdc88b2d0 --- /dev/null +++ b/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