SMTP/BDAT: Fix int/int64_t/uint64_t confusion

The BDAT analyzer should be supporting uint64_t sized chunks reasonably well,
but the ContentLine analyzer does not, And also, I totally got types for
RemainingChunkSize() and in DeliverStream() wrong, resulting in overflows
and segfaults when very large chunk sizes were used.

Tickled by OSS-Fuzz. Actually running the fuzzer locally only took a
few minutes to find the crash, too. Embarrassing.

src/analyzer/protocol/smtp/BDAT.cc

@@ -55,10 +55,16 @@ void SMTP_BDAT_Analyzer::DeliverStream(int len, const u_char* data, bool is_orig
     assert(mail != nullptr);
     assert(! IsFinished());
 
+    // We cast to uint64_t, so need to have a positive value.
+    if ( len < 0 ) {
+        Weird("smtp_bdat_negative_len");
+        return;
+    }
+
     // Upstream analyzer delivers more data than we're
     // expecting for the current chunk. Likely a logic
     // error on their side. Truncate it.
-    if ( len > RemainingChunkSize() ) {
+    if ( static_cast<uint64_t>(len) > RemainingChunkSize() ) {
         Weird("smtp_bdat_chunk_overflow");
         len = static_cast<int>(RemainingChunkSize());
     }
@@ -204,6 +210,20 @@ TEST_CASE("chunk size followed by lastjunk") {
     CHECK(error == std::string("BDAT chunk-size followed by junk"));
 }
 
+TEST_CASE("huge chunk size") {
+    std::string line = "15555555557777777777";
+    const auto& [chunk_size, is_last_chunk, error] = parse_bdat_arg(line.size(), line.c_str());
+    REQUIRE(error == nullptr);
+    CHECK(chunk_size == 15555555557777777777UL);
+}
+
+TEST_CASE("negative chunk size") {
+    std::string line = "-42 LAST";
+    const auto& [chunk_size, is_last_chunk, error] = parse_bdat_arg(line.size(), line.c_str());
+    REQUIRE(error != nullptr);
+    CHECK(error == std::string("BDAT not followed by a valid chunk-size"));
+}
+
 TEST_SUITE_END();
 
 TEST_SUITE_BEGIN("bdat line analyzer");

src/analyzer/protocol/smtp/BDAT.h

@@ -88,7 +88,7 @@ public:
     /**
      * @return The remaining size of the current chunk.
      */
-    int64_t RemainingChunkSize() const { return remaining_chunk_size; }
+    uint64_t RemainingChunkSize() const { return remaining_chunk_size; }
 
     /**
      * @return true if the current chunk was started with LAST.

src/analyzer/protocol/smtp/SMTP.cc

@@ -120,12 +120,15 @@ void SMTP_Analyzer::DeliverStream(int length, const u_char* line, bool orig) {
     // NOTE: do not use IsOrig() here, because of TURN command.
     bool is_sender = orig_is_sender ? orig : ! orig;
 
-    if ( is_sender && bdat ) {
+    if ( length > 0 && is_sender && bdat ) {
         // We're processing BDAT and have switched the ContentLine analyzer
         // into plain mode to send us the full chunk. Ensure we only use up
         // as much as we need in case we get more.
-        int64_t bdat_len = std::min(bdat->RemainingChunkSize(), static_cast<int64_t>(length));
+        int bdat_len = length;
-        if ( bdat->RemainingChunkSize() > 0 )
+        if ( bdat->RemainingChunkSize() < static_cast<uint64_t>(bdat_len) )
+            bdat_len = static_cast<int>(bdat->RemainingChunkSize());
+
+        if ( bdat_len > 0 )
             bdat->NextStream(bdat_len, line, orig);
 
         // All BDAT chunks seen?

testing/btest/Baseline/scripts.base.protocols.bdat-chunk-size-overflow/out

@@ -0,0 +1,20 @@
+### 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, 15555555557777777777 LAST
+smtp_reply, CHhAvVGS1DHFjwGM9, F, 521, BDAT, 5.5.4 Syntax: BDAT count [LAST]
+smtp_request, CHhAvVGS1DHFjwGM9, T, QUIT, 

testing/btest/Baseline/scripts.base.protocols.bdat-chunk-size-overflow/smtp.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	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	41610	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

testing/btest/Baseline/scripts.base.protocols.bdat-chunk-size-overflow/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	41610	127.0.0.1	25	smtp_huge_bdat_chunk	15555555557777777777	F	zeek	SMTP
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.bdat-negative-chunk-size/out

@@ -0,0 +1,20 @@
+### 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, -1 LAST
+smtp_reply, CHhAvVGS1DHFjwGM9, F, 521, BDAT, 5.5.4 Syntax: BDAT count [LAST]
+smtp_request, CHhAvVGS1DHFjwGM9, T, QUIT, 

testing/btest/Baseline/scripts.base.protocols.bdat-negative-chunk-size/smtp.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	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	54028	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

testing/btest/Baseline/scripts.base.protocols.bdat-negative-chunk-size/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	54028	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

testing/btest/scripts/base/protocols/bdat-chunk-size-overflow.test

@@ -0,0 +1,18 @@
+# @TEST-DOC: Test a BDAT line with an overflowing integer size. Pcaps generated with a Python client against Postfix.
+#
+# @TEST-EXEC: zeek -r $TRACES/smtp/smtp-bdat-cmd-chunk-size-overflow.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;
+}

testing/btest/scripts/base/protocols/bdat-negative-chunk-size.test

@@ -0,0 +1,18 @@
+# @TEST-DOC: Test a BDAT line with a negative size. Pcaps generated with a Python client against Postfix.
+#
+# @TEST-EXEC: zeek -r $TRACES/smtp/smtp-bdat-cmd-negative-chunk-size.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;
+}