diff --git a/src/analyzer/protocol/smtp/SMTP.cc b/src/analyzer/protocol/smtp/SMTP.cc index 6ba7838f31..b70dd91ba8 100644 --- a/src/analyzer/protocol/smtp/SMTP.cc +++ b/src/analyzer/protocol/smtp/SMTP.cc @@ -263,31 +263,12 @@ void SMTP_Analyzer::ProcessLine(int length, const char* line, bool orig) { RequestEvent(cmd_len, cmd, data_len, line); } - // For the BDAT command, parse out the chunk-size from the line - // and switch the ContentLineAnalyzer into plain delivery mode - // 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); + // See above, might have already done so. + bool do_update_state = cmd_code != detail::SMTP_CMD_END_OF_DATA; - if ( ! bdat ) { - assert(! mail); - // This is the first BDAT chunk. - BeginData(orig); - bdat = std::make_unique(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); - } - } + if ( cmd_code == detail::SMTP_CMD_BDAT ) + // Do not update state if this isn't a valid BDAT command. + do_update_state = ProcessBdatArg(end_of_line - line, line, orig); else if ( bdat ) { // Non-BDAT command from client but still have BDAT state, // 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(); } - if ( cmd_code != detail::SMTP_CMD_END_OF_DATA ) + if ( do_update_state ) 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 */); } +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(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) { state = detail::SMTP_IN_DATA; skip_data = false; // reset the flag at the beginning of the mail diff --git a/src/analyzer/protocol/smtp/SMTP.h b/src/analyzer/protocol/smtp/SMTP.h index d93c110666..389a45022d 100644 --- a/src/analyzer/protocol/smtp/SMTP.h +++ b/src/analyzer/protocol/smtp/SMTP.h @@ -59,6 +59,7 @@ protected: void NewReply(int reply_code, bool orig); void ProcessExtension(int ext_len, const char* ext); 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); diff --git a/testing/btest/Baseline/scripts.base.protocols.smtp.bdat-cmd-invalid/out b/testing/btest/Baseline/scripts.base.protocols.smtp.bdat-cmd-invalid/out new file mode 100644 index 0000000000..97bc40c19e --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.smtp.bdat-cmd-invalid/out @@ -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: +smtp_reply, CHhAvVGS1DHFjwGM9, F, 250, MAIL, 2.1.0 Ok +smtp_request, CHhAvVGS1DHFjwGM9, T, RCPT, TO: +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: +smtp_reply, ClEkJM2Vm5giqnMf4h, F, 250, MAIL, 2.1.0 Ok +smtp_request, ClEkJM2Vm5giqnMf4h, T, RCPT, TO: +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: +smtp_reply, C4J4Th3PJpwUYZZ6gc, F, 250, MAIL, 2.1.0 Ok +smtp_request, C4J4Th3PJpwUYZZ6gc, T, RCPT, TO: +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, diff --git a/testing/btest/Baseline/scripts.base.protocols.smtp.bdat-cmd-invalid/smtp.log b/testing/btest/Baseline/scripts.base.protocols.smtp.bdat-cmd-invalid/smtp.log new file mode 100644 index 0000000000..243f3ca9e8 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.smtp.bdat-cmd-invalid/smtp.log @@ -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 diff --git a/testing/btest/Baseline/scripts.base.protocols.smtp.bdat-cmd-invalid/weird.log b/testing/btest/Baseline/scripts.base.protocols.smtp.bdat-cmd-invalid/weird.log new file mode 100644 index 0000000000..541c903010 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.smtp.bdat-cmd-invalid/weird.log @@ -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 diff --git a/testing/btest/Traces/smtp/smtp-bdat-cmd-invalid.pcap b/testing/btest/Traces/smtp/smtp-bdat-cmd-invalid.pcap new file mode 100644 index 0000000000..353cfc2d36 Binary files /dev/null and b/testing/btest/Traces/smtp/smtp-bdat-cmd-invalid.pcap differ diff --git a/testing/btest/scripts/base/protocols/smtp/bdat-cmd-invalid.test b/testing/btest/scripts/base/protocols/smtp/bdat-cmd-invalid.test new file mode 100644 index 0000000000..c127d2c14d --- /dev/null +++ b/testing/btest/scripts/base/protocols/smtp/bdat-cmd-invalid.test @@ -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; +}