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

SMTP: No state update for bad BDAT commands

Browse source 
OSS-Fuzz found that providing an invalid BDAT line would tickle an
assert in UpdateState(). The BDAT state was never initialized, but
within UpdateState() that was expected.

This also removes the AnalyzerViolation() call for bad BDAT commands
and instead raises a weird. The SMTP analyzer is very lax and not triggering
the violation allows to parse the server's response to such an invalid
command.

PCAP files produced by a custom Python SMTP client against Postfix.
This commit is contained in:
Arne Welzel 2024-01-15 16:44:49 +01:00
parent 5ad11e00e3
commit ae2a5c83a4
7 changed files with 134 additions and 25 deletions
Download patch file Download diff file Expand all files Collapse all files

56
src/analyzer/protocol/smtp/SMTP.cc
View file

@ -263,31 +263,12 @@ void SMTP_Analyzer::ProcessLine(int length, const char* line, bool orig) {
RequestEvent(cmd_len, cmd, data_len, line); RequestEvent(cmd_len, cmd, data_len, line);
} }
// For the BDAT command, parse out the chunk-size from the line // See above, might have already done so.
// and switch the ContentLineAnalyzer into plain delivery mode bool do_update_state = cmd_code != detail::SMTP_CMD_END_OF_DATA;
// assuming things look valid.
if ( cmd_code == detail::SMTP_CMD_BDAT ) {
const auto [chunk_size, is_last_chunk, error] = detail::parse_bdat_arg(end_of_line - line, line);
if ( ! error ) {
assert(chunk_size >= 0);
auto* cl = orig ? cl_orig : cl_resp;
cl->SetPlainDelivery(chunk_size);
if ( ! bdat ) { if ( cmd_code == detail::SMTP_CMD_BDAT )
assert(! mail); // Do not update state if this isn't a valid BDAT command.
// This is the first BDAT chunk. do_update_state = ProcessBdatArg(end_of_line - line, line, orig);
BeginData(orig);
bdat = std::make_unique<detail::SMTP_BDAT_Analyzer>(Conn(), mail,
zeek::BifConst::SMTP::bdat_max_line_length);
}
bdat->NextChunk(is_last_chunk ? detail::ChunkType::Last : detail::ChunkType::Intermediate,
chunk_size);
}
else {
AnalyzerViolation(error, line, length);
}
}
else if ( bdat ) { else if ( bdat ) {
// Non-BDAT command from client but still have BDAT state, // Non-BDAT command from client but still have BDAT state,
// close it out. This can happen when a client started to // close it out. This can happen when a client started to
@ -297,7 +278,7 @@ void SMTP_Analyzer::ProcessLine(int length, const char* line, bool orig) {
EndData(); EndData();
} }
if ( cmd_code != detail::SMTP_CMD_END_OF_DATA ) if ( do_update_state )
UpdateState(cmd_code, 0, orig); UpdateState(cmd_code, 0, orig);
} }
} }
@ -866,6 +847,31 @@ void SMTP_Analyzer::UnexpectedReply(int cmd_code, int reply_code) {
void SMTP_Analyzer::ProcessData(int length, const char* line) { mail->Deliver(length, line, true /* trailing_CRLF */); } void SMTP_Analyzer::ProcessData(int length, const char* line) { mail->Deliver(length, line, true /* trailing_CRLF */); }
bool SMTP_Analyzer::ProcessBdatArg(int arg_len, const char* arg, bool orig) {
// For the BDAT command, parse out the chunk-size from the line
// and switch the ContentLineAnalyzer into plain delivery mode
// assuming things look valid.
const auto [chunk_size, is_last_chunk, error] = detail::parse_bdat_arg(arg_len, arg);
if ( error ) {
Weird("smtp_invalid_bdat_command", error);
return false;
}
auto* cl = orig ? cl_orig : cl_resp;
cl->SetPlainDelivery(chunk_size);
if ( ! bdat ) {
// This is the first BDAT chunk.
BeginData(orig);
bdat = std::make_unique<detail::SMTP_BDAT_Analyzer>(Conn(), mail, zeek::BifConst::SMTP::bdat_max_line_length);
}
bdat->NextChunk(is_last_chunk ? detail::ChunkType::Last : detail::ChunkType::Intermediate, chunk_size);
// All good.
return true;
}
void SMTP_Analyzer::BeginData(bool orig) { void SMTP_Analyzer::BeginData(bool orig) {
state = detail::SMTP_IN_DATA; state = detail::SMTP_IN_DATA;
skip_data = false; // reset the flag at the beginning of the mail skip_data = false; // reset the flag at the beginning of the mail

1
src/analyzer/protocol/smtp/SMTP.h
View file

@ -59,6 +59,7 @@ protected:
void NewReply(int reply_code, bool orig); void NewReply(int reply_code, bool orig);
void ProcessExtension(int ext_len, const char* ext); void ProcessExtension(int ext_len, const char* ext);
void ProcessData(int length, const char* line); void ProcessData(int length, const char* line);
bool ProcessBdatArg(int arg_len, const char* arg, bool orig);
void UpdateState(int cmd_code, int reply_code, bool orig); void UpdateState(int cmd_code, int reply_code, bool orig);

58
testing/btest/Baseline/scripts.base.protocols.smtp.bdat-cmd-invalid/out Normal file
View file

@ -0,0 +1,58 @@
### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
smtp_reply, CHhAvVGS1DHFjwGM9, F, 220, >, example.com ESMTP Postfix (Debian/GNU)
smtp_request, CHhAvVGS1DHFjwGM9, T, EHLO, localhost
smtp_reply, CHhAvVGS1DHFjwGM9, F, 250, EHLO, example.com
smtp_reply, CHhAvVGS1DHFjwGM9, F, 250, EHLO, PIPELINING
smtp_reply, CHhAvVGS1DHFjwGM9, F, 250, EHLO, SIZE 10240000
smtp_reply, CHhAvVGS1DHFjwGM9, F, 250, EHLO, ETRN
smtp_reply, CHhAvVGS1DHFjwGM9, F, 250, EHLO, STARTTLS
smtp_reply, CHhAvVGS1DHFjwGM9, F, 250, EHLO, ENHANCEDSTATUSCODES
smtp_reply, CHhAvVGS1DHFjwGM9, F, 250, EHLO, 8BITMIME
smtp_reply, CHhAvVGS1DHFjwGM9, F, 250, EHLO, DSN
smtp_reply, CHhAvVGS1DHFjwGM9, F, 250, EHLO, SMTPUTF8
smtp_reply, CHhAvVGS1DHFjwGM9, F, 250, EHLO, CHUNKING
smtp_request, CHhAvVGS1DHFjwGM9, T, MAIL, FROM:<zeek@localhost>
smtp_reply, CHhAvVGS1DHFjwGM9, F, 250, MAIL, 2.1.0 Ok
smtp_request, CHhAvVGS1DHFjwGM9, T, RCPT, TO:<root@localhost>
smtp_reply, CHhAvVGS1DHFjwGM9, F, 250, RCPT, 2.1.5 Ok
smtp_request, CHhAvVGS1DHFjwGM9, T, BDAT,
smtp_reply, CHhAvVGS1DHFjwGM9, F, 521, BDAT, 5.5.4 Syntax: BDAT count [LAST]
smtp_request, CHhAvVGS1DHFjwGM9, T, QUIT,
smtp_reply, ClEkJM2Vm5giqnMf4h, F, 220, >, example.com ESMTP Postfix (Debian/GNU)
smtp_request, ClEkJM2Vm5giqnMf4h, T, EHLO, localhost
smtp_reply, ClEkJM2Vm5giqnMf4h, F, 250, EHLO, example.com
smtp_reply, ClEkJM2Vm5giqnMf4h, F, 250, EHLO, PIPELINING
smtp_reply, ClEkJM2Vm5giqnMf4h, F, 250, EHLO, SIZE 10240000
smtp_reply, ClEkJM2Vm5giqnMf4h, F, 250, EHLO, ETRN
smtp_reply, ClEkJM2Vm5giqnMf4h, F, 250, EHLO, STARTTLS
smtp_reply, ClEkJM2Vm5giqnMf4h, F, 250, EHLO, ENHANCEDSTATUSCODES
smtp_reply, ClEkJM2Vm5giqnMf4h, F, 250, EHLO, 8BITMIME
smtp_reply, ClEkJM2Vm5giqnMf4h, F, 250, EHLO, DSN
smtp_reply, ClEkJM2Vm5giqnMf4h, F, 250, EHLO, SMTPUTF8
smtp_reply, ClEkJM2Vm5giqnMf4h, F, 250, EHLO, CHUNKING
smtp_request, ClEkJM2Vm5giqnMf4h, T, MAIL, FROM:<zeek@localhost>
smtp_reply, ClEkJM2Vm5giqnMf4h, F, 250, MAIL, 2.1.0 Ok
smtp_request, ClEkJM2Vm5giqnMf4h, T, RCPT, TO:<root@localhost>
smtp_reply, ClEkJM2Vm5giqnMf4h, F, 250, RCPT, 2.1.5 Ok
smtp_request, ClEkJM2Vm5giqnMf4h, T, BDAT, 1234 SCRAMBLE
smtp_reply, ClEkJM2Vm5giqnMf4h, F, 521, BDAT, 5.5.4 Syntax: BDAT count [LAST]
smtp_request, ClEkJM2Vm5giqnMf4h, T, QUIT,
smtp_reply, C4J4Th3PJpwUYZZ6gc, F, 220, >, example.com ESMTP Postfix (Debian/GNU)
smtp_request, C4J4Th3PJpwUYZZ6gc, T, EHLO, localhost
smtp_reply, C4J4Th3PJpwUYZZ6gc, F, 250, EHLO, example.com
smtp_reply, C4J4Th3PJpwUYZZ6gc, F, 250, EHLO, PIPELINING
smtp_reply, C4J4Th3PJpwUYZZ6gc, F, 250, EHLO, SIZE 10240000
smtp_reply, C4J4Th3PJpwUYZZ6gc, F, 250, EHLO, ETRN
smtp_reply, C4J4Th3PJpwUYZZ6gc, F, 250, EHLO, STARTTLS
smtp_reply, C4J4Th3PJpwUYZZ6gc, F, 250, EHLO, ENHANCEDSTATUSCODES
smtp_reply, C4J4Th3PJpwUYZZ6gc, F, 250, EHLO, 8BITMIME
smtp_reply, C4J4Th3PJpwUYZZ6gc, F, 250, EHLO, DSN
smtp_reply, C4J4Th3PJpwUYZZ6gc, F, 250, EHLO, SMTPUTF8
smtp_reply, C4J4Th3PJpwUYZZ6gc, F, 250, EHLO, CHUNKING
smtp_request, C4J4Th3PJpwUYZZ6gc, T, MAIL, FROM:<zeek@localhost>
smtp_reply, C4J4Th3PJpwUYZZ6gc, F, 250, MAIL, 2.1.0 Ok
smtp_request, C4J4Th3PJpwUYZZ6gc, T, RCPT, TO:<root@localhost>
smtp_reply, C4J4Th3PJpwUYZZ6gc, F, 250, RCPT, 2.1.5 Ok
smtp_request, C4J4Th3PJpwUYZZ6gc, T, BDAT, SCRAMBLE
smtp_reply, C4J4Th3PJpwUYZZ6gc, F, 521, BDAT, 5.5.4 Syntax: BDAT count [LAST]
smtp_request, C4J4Th3PJpwUYZZ6gc, T, QUIT,

13
testing/btest/Baseline/scripts.base.protocols.smtp.bdat-cmd-invalid/smtp.log Normal file
View file

@ -0,0 +1,13 @@
### 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 smtp
#open XXXX-XX-XX-XX-XX-XX
#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p trans_depth helo mailfrom rcptto date from to cc reply_to msg_id in_reply_to subject x_originating_ip first_received second_received last_reply path user_agent tls fuids
#types time string addr port addr port count string string set[string] string string set[string] set[string] string string string string addr string string string vector[addr] string bool vector[string]
XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 127.0.0.1 33472 127.0.0.1 25 1 localhost zeek@localhost root@localhost - - - - - - - - - - - 521 5.5.4 Syntax: BDAT count [LAST] 127.0.0.1,127.0.0.1 - F (empty)
XXXXXXXXXX.XXXXXX ClEkJM2Vm5giqnMf4h 127.0.0.1 52364 127.0.0.1 25 1 localhost zeek@localhost root@localhost - - - - - - - - - - - 521 5.5.4 Syntax: BDAT count [LAST] 127.0.0.1,127.0.0.1 - F (empty)
XXXXXXXXXX.XXXXXX C4J4Th3PJpwUYZZ6gc 127.0.0.1 46862 127.0.0.1 25 1 localhost zeek@localhost root@localhost - - - - - - - - - - - 521 5.5.4 Syntax: BDAT count [LAST] 127.0.0.1,127.0.0.1 - F (empty)
#close XXXX-XX-XX-XX-XX-XX

13
testing/btest/Baseline/scripts.base.protocols.smtp.bdat-cmd-invalid/weird.log Normal file
View file

@ -0,0 +1,13 @@
### 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 33472 127.0.0.1 25 smtp_invalid_bdat_command BDAT not followed by a valid chunk-size F zeek SMTP
XXXXXXXXXX.XXXXXX ClEkJM2Vm5giqnMf4h 127.0.0.1 52364 127.0.0.1 25 smtp_invalid_bdat_command BDAT chunk-size followed by junk F zeek SMTP
XXXXXXXXXX.XXXXXX C4J4Th3PJpwUYZZ6gc 127.0.0.1 46862 127.0.0.1 25 smtp_invalid_bdat_command BDAT not followed by a valid chunk-size F zeek SMTP
#close XXXX-XX-XX-XX-XX-XX

BIN
testing/btest/Traces/smtp/smtp-bdat-cmd-invalid.pcap Normal file
View file

Binary file not shown.

18
testing/btest/scripts/base/protocols/smtp/bdat-cmd-invalid.test Normal file
View file

@ -0,0 +1,18 @@
# @TEST-DOC: Test invalid BDAT lines. Pcaps generated with a Python client against Postfix.
#
# @TEST-EXEC: zeek -b -r $TRACES/smtp/smtp-bdat-cmd-invalid.pcap %INPUT >out
# @TEST-EXEC: btest-diff smtp.log
# @TEST-EXEC: btest-diff weird.log
# @TEST-EXEC: btest-diff out
@load base/protocols/conn
@load base/protocols/smtp
event smtp_request(c: connection, is_orig: bool, command: string, arg: string) {
print "smtp_request", c$uid, is_orig, command, arg;
}
event smtp_reply(c: connection, is_orig: bool, code: count, cmd: string,
msg: string, cont_resp: bool) {
print "smtp_reply", c$uid, is_orig, code, cmd, msg;
}