diff --git a/src/analyzer/protocol/smtp/BDAT.cc b/src/analyzer/protocol/smtp/BDAT.cc index ddb02f525f..999b4dfbcb 100644 --- a/src/analyzer/protocol/smtp/BDAT.cc +++ b/src/analyzer/protocol/smtp/BDAT.cc @@ -18,13 +18,20 @@ struct BDATCmd parse_bdat_arg(int length, const char* arg) { return r; } + errno = 0; char* chunk_size_end = nullptr; - uint64_t chunk_size = strtoul(arg, &chunk_size_end, 10); + uint64_t chunk_size = strtoull(arg, &chunk_size_end, 10); if ( *chunk_size_end != ' ' && chunk_size_end != arg_end ) { r.error = "BDAT chunk-size not valid"; return r; } + // strtoull() returns ULLONG_MAX and sets errno on overflow. + if ( chunk_size == ULLONG_MAX && errno == ERANGE ) { + r.error = "BDAT chunk-size too large"; + return r; + } + r.chunk_size = chunk_size; // If there's something left after the chunk-size, @@ -217,6 +224,14 @@ TEST_CASE("huge chunk size") { CHECK(chunk_size == 15555555557777777777UL); } +TEST_CASE("UINT64_MAX * 10 chunk size") { + // UINT64_MAX is 18446744073709551615UL, multiply by 10 + std::string line = "184467440737095516150"; + const auto& [chunk_size, is_last_chunk, error] = parse_bdat_arg(line.size(), line.c_str()); + REQUIRE(error != nullptr); + CHECK(error == std::string("BDAT chunk-size too large")); +} + 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()); diff --git a/testing/btest/Baseline/scripts.base.protocols.bdat-chunk-size-overflow2/out b/testing/btest/Baseline/scripts.base.protocols.bdat-chunk-size-overflow2/out new file mode 100644 index 0000000000..c2f0deea57 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.bdat-chunk-size-overflow2/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: +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, 184467440737095516150 LAST +smtp_reply, CHhAvVGS1DHFjwGM9, F, 521, BDAT, 5.5.4 Syntax: BDAT count [LAST] +smtp_request, CHhAvVGS1DHFjwGM9, T, QUIT, diff --git a/testing/btest/Baseline/scripts.base.protocols.bdat-chunk-size-overflow2/smtp.log b/testing/btest/Baseline/scripts.base.protocols.bdat-chunk-size-overflow2/smtp.log new file mode 100644 index 0000000000..bbfab72dc2 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.bdat-chunk-size-overflow2/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 33762 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.bdat-chunk-size-overflow2/weird.log b/testing/btest/Baseline/scripts.base.protocols.bdat-chunk-size-overflow2/weird.log new file mode 100644 index 0000000000..2129c89e16 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.bdat-chunk-size-overflow2/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 33762 127.0.0.1 25 smtp_invalid_bdat_command BDAT chunk-size too large F zeek SMTP +#close XXXX-XX-XX-XX-XX-XX diff --git a/testing/btest/Traces/smtp/smtp-bdat-cmd-chunk-size-overflow2.pcap b/testing/btest/Traces/smtp/smtp-bdat-cmd-chunk-size-overflow2.pcap new file mode 100644 index 0000000000..660c698e97 Binary files /dev/null and b/testing/btest/Traces/smtp/smtp-bdat-cmd-chunk-size-overflow2.pcap differ diff --git a/testing/btest/scripts/base/protocols/bdat-chunk-size-overflow2.test b/testing/btest/scripts/base/protocols/bdat-chunk-size-overflow2.test new file mode 100644 index 0000000000..3be2dab562 --- /dev/null +++ b/testing/btest/scripts/base/protocols/bdat-chunk-size-overflow2.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-overflow2.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; +}