Mirror/zeek
1
0
Fork 0
mirror of https://github.com/zeek/zeek.git synced 2025-10-02 06:38:20 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

POP3: Rework unbounded pending command fix

Browse source 
Processing out-of-order commands or finishing commands based on invalid
server responses resulted in inconsistent analyzer state, potentially
triggering null pointer references for crafted traffic.

This commit reworks cf9fe91705 such that
too many pending commands are simply discarded, rather than any attempt
being made to process them. Further, invalid server responses do not
result in command completion anymore.

Test PCAP was crafted based on traffic produced by the OSS-Fuzz reproducer.

Closes #215
This commit is contained in:
Arne Welzel 2024-09-28 16:09:40 +02:00 • committed by Christian Kreibich
parent d70bb6a889
commit 4656faed6c
10 changed files with 59 additions and 15 deletions
Download patch file Download diff file Expand all files Collapse all files

12
src/analyzer/protocol/pop3/POP3.cc
View file

@ -206,16 +206,14 @@ void POP3_Analyzer::ProcessRequest(int length, const char* line) {
cmds.emplace_back(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();
}
}
@ -583,10 +581,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;
}

11
testing/btest/Baseline/scripts.base.protocols.pop3.bad-list-retr-crafted/analyzer.log Normal file
View file

@ -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

11
testing/btest/Baseline/scripts.base.protocols.pop3.bad-list-retr-crafted/conn.log Normal file
View file

@ -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

11
testing/btest/Baseline/scripts.base.protocols.pop3.bad-list-retr-crafted/weird.log Normal file
View file

@ -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

10
testing/btest/Baseline/scripts.base.protocols.pop3.redis/analyzer.log
View file

@ -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

2
testing/btest/Baseline/scripts.base.protocols.pop3.redis/out
View file

@ -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,

1
testing/btest/Baseline/scripts.base.protocols.pop3.redis/weird.log
View file

@ -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

BIN
testing/btest/Traces/pop3/bad-list-retr-crafted.pcap Normal file
View file

Binary file not shown.

14
testing/btest/scripts/base/protocols/pop3/bad-list-retr-crafted.zeek Normal file
View file

@ -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);
}

2
testing/btest/scripts/base/protocols/pop3/redis.zeek
View file

@ -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;