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: Prevent unbounded state growth

Browse source 
The cmds list may grow unbounded due to the POP3 analyzer being in
multiLine mode after seeing `AUTH` in a Redis connection, but never
a `.` terminator. This can easily be provoked by the Redis ping
command.

This adds two heuristics: 1) Forcefully process the oldest commands in
the cmds list and cap it at max_pending_commands. 2) Start raising
analyzer violations if the client has been using more than
max_unknown_client_commands commands (default 10).

Closes #3936
This commit is contained in:
Arne Welzel 2024-09-18 17:31:58 +02:00
parent b4fdce8d5b
commit cf9fe91705
16 changed files with 130 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

5
NEWS
View file

@ -74,6 +74,11 @@ Changed Functionality
* The ``mysql.log`` for user change commands will contain *just* the username
instead of the remaining parts of the command, including auth plugin data.
* The POP3 parser has been hardened to avoid unbounded state growth in the
face of one-sided traffic capture or when enabled for non-POP3 traffic.
Concretely, the Redis protocol's AUTH mechanism enables the POP3 analyzer
for such connections through DPD.
Removed Functionality
---------------------

16
scripts/base/init-bare.zeek
View file

@ -2903,6 +2903,22 @@ export {
} # end export
module POP3;
export {
## How many commands a POP3 client may have pending
## before Zeek forcefully removes the oldest.
##
## Setting this value to 0 removes the limit.
const max_pending_commands = 10 &redef;
## How many invalid commands a POP3 client may use
## before Zeek starts raising analyzer violations.
##
## Setting this value to 0 removes the limit.
const max_unknown_client_commands = 10 &redef;
} # end export
module Threading;

1
src/analyzer/protocol/pop3/CMakeLists.txt
View file

@ -5,4 +5,5 @@ zeek_add_plugin(
POP3.cc
Plugin.cc
BIFS
consts.bif
events.bif)

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

@ -12,6 +12,7 @@
#include "zeek/Base64.h"
#include "zeek/Reporter.h"
#include "zeek/analyzer/Manager.h"
#include "zeek/analyzer/protocol/pop3/consts.bif.h"
#include "zeek/analyzer/protocol/pop3/events.bif.h"
namespace zeek::analyzer::pop3 {
@ -41,6 +42,7 @@ POP3_Analyzer::POP3_Analyzer(Connection* conn) : analyzer::tcp::TCP_ApplicationA
authLines = 0;
mail = nullptr;
unknown_client_cmds = 0;
cl_orig = new analyzer::tcp::ContentLine_Analyzer(conn, true);
AddSupportAnalyzer(cl_orig);
@ -205,6 +207,21 @@ void POP3_Analyzer::ProcessRequest(int length, const char* line) {
// keep a list of pending commands.
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.
//
// This may be caused by packet drops, one-sided traffic, 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();
}
}
if ( cmds.size() == 1 )
// Not waiting for another server response,
// so we can process it immediately.
@ -236,10 +253,19 @@ void POP3_Analyzer::ProcessClientCmd() {
if ( cmd_code == -1 ) {
if ( ! waitingForAuthentication ) {
Weird("pop3_client_command_unknown");
Weird("pop3_client_command_unknown", (tokens.size() > 0 ? tokens[0].c_str() : "???"));
if ( subState == detail::POP3_WOK )
subState = detail::POP3_OK;
++unknown_client_cmds;
if ( zeek::BifConst::POP3::max_unknown_client_commands > 0 ) {
if ( unknown_client_cmds > zeek::BifConst::POP3::max_unknown_client_commands ) {
AnalyzerViolation("too many unknown client commands");
}
}
}
return;
}
@ -299,6 +325,7 @@ void POP3_Analyzer::ProcessClientCmd() {
if ( masterState == detail::POP3_AUTHORIZATION ) {
POP3Event(pop3_request, true, cmd, message);
if ( ! *message ) {
// This is the client requesting a list of AUTH mechanisms available.
requestForMultiLine = true;
state = detail::AUTH;
subState = detail::POP3_WOK;
@ -555,9 +582,13 @@ void POP3_Analyzer::ProcessReply(int length, const char* line) {
AnalyzerViolation(util::fmt("unknown server command (%s)", (tokens.size() > 0 ? tokens[0].c_str() : "???")),
line, length);
Weird("pop3_server_command_unknown");
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;
}

1
src/analyzer/protocol/pop3/POP3.h
View file

@ -106,6 +106,7 @@ protected:
analyzer::mime::MIME_Mail* mail;
std::list<std::string> cmds;
zeek_uint_t unknown_client_cmds;
private:
bool tls;

2
src/analyzer/protocol/pop3/consts.bif Normal file
View file

@ -0,0 +1,2 @@
const POP3::max_pending_commands: count;
const POP3::max_unknown_client_commands: count;

2
testing/btest/Baseline/core.max-analyzer-violations/weird.log
View file

@ -7,7 +7,7 @@
#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 58246 127.0.0.1 110 pop3_server_command_unknown - F zeek POP3
XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 127.0.0.1 58246 127.0.0.1 110 pop3_server_command_unknown + F zeek POP3
XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 127.0.0.1 58246 127.0.0.1 110 line_terminated_with_single_CR - F zeek CONTENTLINE
XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 127.0.0.1 58246 127.0.0.1 110 too_many_analyzer_violations - F zeek POP3
#close XXXX-XX-XX-XX-XX-XX

1
testing/btest/Baseline/coverage.bare-load-baseline/canonified_loaded_scripts.log
View file

@ -195,6 +195,7 @@ scripts/base/init-frameworks-and-bifs.zeek
build/scripts/base/bif/plugins/Zeek_NTLM.events.bif.zeek
build/scripts/base/bif/plugins/Zeek_NTP.types.bif.zeek
build/scripts/base/bif/plugins/Zeek_NTP.events.bif.zeek
build/scripts/base/bif/plugins/Zeek_POP3.consts.bif.zeek
build/scripts/base/bif/plugins/Zeek_POP3.events.bif.zeek
build/scripts/base/bif/plugins/Zeek_RADIUS.events.bif.zeek
build/scripts/base/bif/plugins/Zeek_RDP.events.bif.zeek

1
testing/btest/Baseline/coverage.default-load-baseline/canonified_loaded_scripts.log
View file

@ -195,6 +195,7 @@ scripts/base/init-frameworks-and-bifs.zeek
build/scripts/base/bif/plugins/Zeek_NTLM.events.bif.zeek
build/scripts/base/bif/plugins/Zeek_NTP.types.bif.zeek
build/scripts/base/bif/plugins/Zeek_NTP.events.bif.zeek
build/scripts/base/bif/plugins/Zeek_POP3.consts.bif.zeek
build/scripts/base/bif/plugins/Zeek_POP3.events.bif.zeek
build/scripts/base/bif/plugins/Zeek_RADIUS.events.bif.zeek
build/scripts/base/bif/plugins/Zeek_RDP.events.bif.zeek

6
testing/btest/Baseline/plugins.hooks/output
View file

@ -385,6 +385,7 @@
0.000000 MetaHookPost LoadFile(0, ./Zeek_NetBIOS.functions.bif.zeek, <...>/Zeek_NetBIOS.functions.bif.zeek) -> -1
0.000000 MetaHookPost LoadFile(0, ./Zeek_NoneWriter.none.bif.zeek, <...>/Zeek_NoneWriter.none.bif.zeek) -> -1
0.000000 MetaHookPost LoadFile(0, ./Zeek_PE.events.bif.zeek, <...>/Zeek_PE.events.bif.zeek) -> -1
0.000000 MetaHookPost LoadFile(0, ./Zeek_POP3.consts.bif.zeek, <...>/Zeek_POP3.consts.bif.zeek) -> -1
0.000000 MetaHookPost LoadFile(0, ./Zeek_POP3.events.bif.zeek, <...>/Zeek_POP3.events.bif.zeek) -> -1
0.000000 MetaHookPost LoadFile(0, ./Zeek_RADIUS.events.bif.zeek, <...>/Zeek_RADIUS.events.bif.zeek) -> -1
0.000000 MetaHookPost LoadFile(0, ./Zeek_RDP.events.bif.zeek, <...>/Zeek_RDP.events.bif.zeek) -> -1
@ -682,6 +683,7 @@
0.000000 MetaHookPost LoadFileExtended(0, ./Zeek_NetBIOS.functions.bif.zeek, <...>/Zeek_NetBIOS.functions.bif.zeek) -> (-1, <no content>)
0.000000 MetaHookPost LoadFileExtended(0, ./Zeek_NoneWriter.none.bif.zeek, <...>/Zeek_NoneWriter.none.bif.zeek) -> (-1, <no content>)
0.000000 MetaHookPost LoadFileExtended(0, ./Zeek_PE.events.bif.zeek, <...>/Zeek_PE.events.bif.zeek) -> (-1, <no content>)
0.000000 MetaHookPost LoadFileExtended(0, ./Zeek_POP3.consts.bif.zeek, <...>/Zeek_POP3.consts.bif.zeek) -> (-1, <no content>)
0.000000 MetaHookPost LoadFileExtended(0, ./Zeek_POP3.events.bif.zeek, <...>/Zeek_POP3.events.bif.zeek) -> (-1, <no content>)
0.000000 MetaHookPost LoadFileExtended(0, ./Zeek_RADIUS.events.bif.zeek, <...>/Zeek_RADIUS.events.bif.zeek) -> (-1, <no content>)
0.000000 MetaHookPost LoadFileExtended(0, ./Zeek_RDP.events.bif.zeek, <...>/Zeek_RDP.events.bif.zeek) -> (-1, <no content>)
@ -1311,6 +1313,7 @@
0.000000 MetaHookPre LoadFile(0, ./Zeek_NetBIOS.functions.bif.zeek, <...>/Zeek_NetBIOS.functions.bif.zeek)
0.000000 MetaHookPre LoadFile(0, ./Zeek_NoneWriter.none.bif.zeek, <...>/Zeek_NoneWriter.none.bif.zeek)
0.000000 MetaHookPre LoadFile(0, ./Zeek_PE.events.bif.zeek, <...>/Zeek_PE.events.bif.zeek)
0.000000 MetaHookPre LoadFile(0, ./Zeek_POP3.consts.bif.zeek, <...>/Zeek_POP3.consts.bif.zeek)
0.000000 MetaHookPre LoadFile(0, ./Zeek_POP3.events.bif.zeek, <...>/Zeek_POP3.events.bif.zeek)
0.000000 MetaHookPre LoadFile(0, ./Zeek_RADIUS.events.bif.zeek, <...>/Zeek_RADIUS.events.bif.zeek)
0.000000 MetaHookPre LoadFile(0, ./Zeek_RDP.events.bif.zeek, <...>/Zeek_RDP.events.bif.zeek)
@ -1608,6 +1611,7 @@
0.000000 MetaHookPre LoadFileExtended(0, ./Zeek_NetBIOS.functions.bif.zeek, <...>/Zeek_NetBIOS.functions.bif.zeek)
0.000000 MetaHookPre LoadFileExtended(0, ./Zeek_NoneWriter.none.bif.zeek, <...>/Zeek_NoneWriter.none.bif.zeek)
0.000000 MetaHookPre LoadFileExtended(0, ./Zeek_PE.events.bif.zeek, <...>/Zeek_PE.events.bif.zeek)
0.000000 MetaHookPre LoadFileExtended(0, ./Zeek_POP3.consts.bif.zeek, <...>/Zeek_POP3.consts.bif.zeek)
0.000000 MetaHookPre LoadFileExtended(0, ./Zeek_POP3.events.bif.zeek, <...>/Zeek_POP3.events.bif.zeek)
0.000000 MetaHookPre LoadFileExtended(0, ./Zeek_RADIUS.events.bif.zeek, <...>/Zeek_RADIUS.events.bif.zeek)
0.000000 MetaHookPre LoadFileExtended(0, ./Zeek_RDP.events.bif.zeek, <...>/Zeek_RDP.events.bif.zeek)
@ -2236,6 +2240,7 @@
0.000000 | HookLoadFile ./Zeek_NetBIOS.functions.bif.zeek <...>/Zeek_NetBIOS.functions.bif.zeek
0.000000 | HookLoadFile ./Zeek_NoneWriter.none.bif.zeek <...>/Zeek_NoneWriter.none.bif.zeek
0.000000 | HookLoadFile ./Zeek_PE.events.bif.zeek <...>/Zeek_PE.events.bif.zeek
0.000000 | HookLoadFile ./Zeek_POP3.consts.bif.zeek <...>/Zeek_POP3.consts.bif.zeek
0.000000 | HookLoadFile ./Zeek_POP3.events.bif.zeek <...>/Zeek_POP3.events.bif.zeek
0.000000 | HookLoadFile ./Zeek_RADIUS.events.bif.zeek <...>/Zeek_RADIUS.events.bif.zeek
0.000000 | HookLoadFile ./Zeek_RDP.events.bif.zeek <...>/Zeek_RDP.events.bif.zeek
@ -2533,6 +2538,7 @@
0.000000 | HookLoadFileExtended ./Zeek_NetBIOS.functions.bif.zeek <...>/Zeek_NetBIOS.functions.bif.zeek
0.000000 | HookLoadFileExtended ./Zeek_NoneWriter.none.bif.zeek <...>/Zeek_NoneWriter.none.bif.zeek
0.000000 | HookLoadFileExtended ./Zeek_PE.events.bif.zeek <...>/Zeek_PE.events.bif.zeek
0.000000 | HookLoadFileExtended ./Zeek_POP3.consts.bif.zeek <...>/Zeek_POP3.consts.bif.zeek
0.000000 | HookLoadFileExtended ./Zeek_POP3.events.bif.zeek <...>/Zeek_POP3.events.bif.zeek
0.000000 | HookLoadFileExtended ./Zeek_RADIUS.events.bif.zeek <...>/Zeek_RADIUS.events.bif.zeek
0.000000 | HookLoadFileExtended ./Zeek_RDP.events.bif.zeek <...>/Zeek_RDP.events.bif.zeek

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

@ -0,0 +1,16 @@
### 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 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 -
#close XXXX-XX-XX-XX-XX-XX

11
testing/btest/Baseline/scripts.base.protocols.pop3.redis/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 59954 127.0.0.1 6379 tcp - 0.002030 848 370 SF T T 0 ShADadfF 58 3872 58 3394 -
#close XXXX-XX-XX-XX-XX-XX

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

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

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

@ -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 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 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
#close XXXX-XX-XX-XX-XX-XX

BIN
testing/btest/Traces/pop3/redis-50-pings.pcap Normal file
View file

Binary file not shown.

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

@ -0,0 +1,20 @@
# @TEST-DOC: The POP3 signature triggered on Redis traffic. Ensure the analyzer is eventually removed to avoid.
# @TEST-EXEC: zeek -C -b -r $TRACES/pop3/redis-50-pings.pcap %INPUT >out
# @TEST-EXEC: btest-diff conn.log
# @TEST-EXEC: btest-diff out
# @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 pop3_request(c: connection, is_orig: bool, cmd: string, arg: string)
{
print c$uid, "pop3_request", is_orig, cmd, arg;
}
event pop3_reply(c: connection, is_orig: bool, cmd: string, arg: string)
{
print c$uid, "pop3_reply", is_orig, cmd, arg;
}