diff --git a/src/analyzer/protocol/pop3/POP3.cc b/src/analyzer/protocol/pop3/POP3.cc index 0d46a0e847..5ad6950e27 100644 --- a/src/analyzer/protocol/pop3/POP3.cc +++ b/src/analyzer/protocol/pop3/POP3.cc @@ -227,18 +227,16 @@ void POP3_Analyzer::ProcessRequest(int length, const char* line) cmds.push_back(std::string(line)); // Prevent unbounded state growth of cmds if there are no matching - // server replies by just processing commands even if we didn't see - // the server response. + // server replies by simply dropping the oldest command. // - // This may be caused by packet drops, one-sided traffic, analyzing - // the wrong protocol (Redis), etc. + // This may be caused by packet drops of the server side, one-sided + // traffic, or analyzing the wrong protocol (Redis), etc. if ( zeek::BifConst::POP3::max_pending_commands > 0 ) { if ( cmds.size() > zeek::BifConst::POP3::max_pending_commands ) { Weird("pop3_client_too_many_pending_commands"); - ProcessClientCmd(); cmds.pop_front(); } } @@ -660,10 +658,6 @@ void POP3_Analyzer::ProcessReply(int length, const char* line) Weird("pop3_server_command_unknown", (tokens.size() > 0 ? tokens[0].c_str() : "???")); if ( subState == detail::POP3_WOK ) subState = detail::POP3_OK; - - // If we're not in state AUTH and receive "some" response, - // assume it was for the last command from the client. - FinishClientCmd(); } return; diff --git a/testing/btest/Baseline/scripts.base.protocols.pop3.bad-list-retr-crafted/analyzer.log b/testing/btest/Baseline/scripts.base.protocols.pop3.bad-list-retr-crafted/analyzer.log new file mode 100644 index 0000000000..23f62ab8e7 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.pop3.bad-list-retr-crafted/analyzer.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 analyzer +#open XXXX-XX-XX-XX-XX-XX +#fields ts cause analyzer_kind analyzer_name uid fuid id.orig_h id.orig_p id.resp_h id.resp_p failure_reason failure_data +#types time string string string string string addr port addr port string string +XXXXXXXXXX.XXXXXX violation protocol POP3 CHhAvVGS1DHFjwGM9 - 127.0.0.1 58854 127.0.0.1 110 unknown server command (GARBAGE) GARBAGE (and LIST response missing .) +#close XXXX-XX-XX-XX-XX-XX diff --git a/testing/btest/Baseline/scripts.base.protocols.pop3.bad-list-retr-crafted/conn.log b/testing/btest/Baseline/scripts.base.protocols.pop3.bad-list-retr-crafted/conn.log new file mode 100644 index 0000000000..1ead1294bf --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.pop3.bad-list-retr-crafted/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 58854 127.0.0.1 110 tcp - 0.151387 20 253 RSTO T T 0 ShAdDaFR 20 1056 16 1093 - +#close XXXX-XX-XX-XX-XX-XX diff --git a/testing/btest/Baseline/scripts.base.protocols.pop3.bad-list-retr-crafted/weird.log b/testing/btest/Baseline/scripts.base.protocols.pop3.bad-list-retr-crafted/weird.log new file mode 100644 index 0000000000..b661991b9e --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.pop3.bad-list-retr-crafted/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 58854 127.0.0.1 110 pop3_server_command_unknown GARBAGE F zeek POP3 +#close XXXX-XX-XX-XX-XX-XX diff --git a/testing/btest/Baseline/scripts.base.protocols.pop3.redis/analyzer.log b/testing/btest/Baseline/scripts.base.protocols.pop3.redis/analyzer.log index 3c1d391690..3f9f9ae848 100644 --- a/testing/btest/Baseline/scripts.base.protocols.pop3.redis/analyzer.log +++ b/testing/btest/Baseline/scripts.base.protocols.pop3.redis/analyzer.log @@ -8,9 +8,9 @@ #fields ts cause analyzer_kind analyzer_name uid fuid id.orig_h id.orig_p id.resp_h id.resp_p failure_reason failure_data #types time string string string string string addr port addr port string string XXXXXXXXXX.XXXXXX violation protocol POP3 CHhAvVGS1DHFjwGM9 - 127.0.0.1 59954 127.0.0.1 6379 too many unknown client commands - -XXXXXXXXXX.XXXXXX violation protocol POP3 CHhAvVGS1DHFjwGM9 - 127.0.0.1 59954 127.0.0.1 6379 too many unknown client commands - -XXXXXXXXXX.XXXXXX violation protocol POP3 CHhAvVGS1DHFjwGM9 - 127.0.0.1 59954 127.0.0.1 6379 too many unknown client commands - -XXXXXXXXXX.XXXXXX violation protocol POP3 CHhAvVGS1DHFjwGM9 - 127.0.0.1 59954 127.0.0.1 6379 too many unknown client commands - -XXXXXXXXXX.XXXXXX violation protocol POP3 CHhAvVGS1DHFjwGM9 - 127.0.0.1 59954 127.0.0.1 6379 too many unknown client commands - -XXXXXXXXXX.XXXXXX violation protocol POP3 CHhAvVGS1DHFjwGM9 - 127.0.0.1 59954 127.0.0.1 6379 too many unknown client commands - +XXXXXXXXXX.XXXXXX violation protocol POP3 CHhAvVGS1DHFjwGM9 - 127.0.0.1 59954 127.0.0.1 6379 unknown server command (+PONG) +PONG +XXXXXXXXXX.XXXXXX violation protocol POP3 CHhAvVGS1DHFjwGM9 - 127.0.0.1 59954 127.0.0.1 6379 unknown server command (+PONG) +PONG +XXXXXXXXXX.XXXXXX violation protocol POP3 CHhAvVGS1DHFjwGM9 - 127.0.0.1 59954 127.0.0.1 6379 unknown server command (+PONG) +PONG +XXXXXXXXXX.XXXXXX violation protocol POP3 CHhAvVGS1DHFjwGM9 - 127.0.0.1 59954 127.0.0.1 6379 unknown server command (+PONG) +PONG +XXXXXXXXXX.XXXXXX violation protocol POP3 CHhAvVGS1DHFjwGM9 - 127.0.0.1 59954 127.0.0.1 6379 unknown server command (+PONG) +PONG #close XXXX-XX-XX-XX-XX-XX diff --git a/testing/btest/Baseline/scripts.base.protocols.pop3.redis/out b/testing/btest/Baseline/scripts.base.protocols.pop3.redis/out index de83d41ef3..fe3bec3f1d 100644 --- a/testing/btest/Baseline/scripts.base.protocols.pop3.redis/out +++ b/testing/btest/Baseline/scripts.base.protocols.pop3.redis/out @@ -1,4 +1,4 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. CHhAvVGS1DHFjwGM9, pop3_reply, F, OK, -CHhAvVGS1DHFjwGM9, pop3_request, T, AUTH, +CHhAvVGS1DHFjwGM9, pop3_reply, F, OK, CHhAvVGS1DHFjwGM9, pop3_reply, F, OK, diff --git a/testing/btest/Baseline/scripts.base.protocols.pop3.redis/weird.log b/testing/btest/Baseline/scripts.base.protocols.pop3.redis/weird.log index 36a176032c..7ac280b3c0 100644 --- a/testing/btest/Baseline/scripts.base.protocols.pop3.redis/weird.log +++ b/testing/btest/Baseline/scripts.base.protocols.pop3.redis/weird.log @@ -9,4 +9,5 @@ #types time string addr port addr port string string bool string string XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 127.0.0.1 59954 127.0.0.1 6379 pop3_client_command_unknown *2 F zeek POP3 XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 127.0.0.1 59954 127.0.0.1 6379 pop3_client_too_many_pending_commands - F zeek POP3 +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 127.0.0.1 59954 127.0.0.1 6379 pop3_server_command_unknown +PONG F zeek POP3 #close XXXX-XX-XX-XX-XX-XX diff --git a/testing/btest/Traces/pop3/bad-list-retr-crafted.pcap b/testing/btest/Traces/pop3/bad-list-retr-crafted.pcap new file mode 100644 index 0000000000..7543e833e1 Binary files /dev/null and b/testing/btest/Traces/pop3/bad-list-retr-crafted.pcap differ diff --git a/testing/btest/scripts/base/protocols/pop3/bad-list-retr-crafted.zeek b/testing/btest/scripts/base/protocols/pop3/bad-list-retr-crafted.zeek new file mode 100644 index 0000000000..2aedeac81e --- /dev/null +++ b/testing/btest/scripts/base/protocols/pop3/bad-list-retr-crafted.zeek @@ -0,0 +1,14 @@ +# @TEST-DOC: Crafted pcap causing crashes due to mail not initialized. +# @TEST-EXEC: zeek -b -r $TRACES/pop3/bad-list-retr-crafted.pcap %INPUT +# @TEST-EXEC: btest-diff conn.log +# @TEST-EXEC: btest-diff weird.log +# @TEST-EXEC: btest-diff analyzer.log + +@load base/frameworks/notice/weird +@load base/protocols/conn +@load base/protocols/pop3 + +event zeek_init() + { + Analyzer::register_for_port(Analyzer::ANALYZER_POP3, 110/tcp); + } diff --git a/testing/btest/scripts/base/protocols/pop3/redis.zeek b/testing/btest/scripts/base/protocols/pop3/redis.zeek index d19845aae2..137baf459c 100644 --- a/testing/btest/scripts/base/protocols/pop3/redis.zeek +++ b/testing/btest/scripts/base/protocols/pop3/redis.zeek @@ -9,6 +9,8 @@ @load base/protocols/conn @load base/protocols/pop3 +redef POP3::max_unknown_client_commands = 3; + event pop3_request(c: connection, is_orig: bool, cmd: string, arg: string) { print c$uid, "pop3_request", is_orig, cmd, arg;