From e1ed7092430ac06af1da3b1b0e1f230c1767bb97 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Fri, 19 Jan 2024 13:22:57 +0100 Subject: [PATCH] SMTP/BDAT: Use strtoull and bail on UULONG_MAX values --- src/analyzer/protocol/smtp/BDAT.cc | 17 ++++++++++++++- .../out | 20 ++++++++++++++++++ .../smtp.log | 11 ++++++++++ .../weird.log | 11 ++++++++++ .../smtp-bdat-cmd-chunk-size-overflow2.pcap | Bin 0 -> 2215 bytes .../protocols/bdat-chunk-size-overflow2.test | 18 ++++++++++++++++ 6 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 testing/btest/Baseline/scripts.base.protocols.bdat-chunk-size-overflow2/out create mode 100644 testing/btest/Baseline/scripts.base.protocols.bdat-chunk-size-overflow2/smtp.log create mode 100644 testing/btest/Baseline/scripts.base.protocols.bdat-chunk-size-overflow2/weird.log create mode 100644 testing/btest/Traces/smtp/smtp-bdat-cmd-chunk-size-overflow2.pcap create mode 100644 testing/btest/scripts/base/protocols/bdat-chunk-size-overflow2.test 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 0000000000000000000000000000000000000000..660c698e97b407aafba0b9ef807339723080d9ce GIT binary patch literal 2215 zcmaKsTTD}T9LN8s;APdKbsE7-dLA~SaqD`zQ-KDRmL8$lo9(G_UKUF!mk87#DEgrF z@&FH#%q23$n1}&%AqxSsC42F~%)RW%xMbO*8^lLn@L^ew`~80B{7+65`%C^wd)o8) zUVc4&{$$~vjg>P#Z{9E_z?*$tKjf~(9jpajY4Q6`Ib?4mKe2=K4qH01++sJD<&TcR~i;vmX@R@7XmnsrZi*hpV8wV@K8@zN~GcGz*+> z^wSRg$kY$~C+W{O@g;Qpq)A|>0ENdoM2u;hSM5h9LO+Mjo5VJ7EF~Kgs;I{O^?mt# z|2yrpUlB^R^DVWrgS$7{k)fRew$tP^wfaD-jkn`7+qnSz62BfNWqJD_y8!J3v7PTv z7V-PAoj*f1)&{lUu@1F!Oxr@lTimhHw@!q1w%;^~)8M!-7z(JOR4dE-Yh>9|^LdS@ z&gYZ%OR(lhAf-C=9cNo?tOLaGScf#faE((-Hm+&36oi(hu%+8sQ*#D1r+r>Y^tpX* zSxjB(?avJQMRJkN5A^nnCsFpaE_(%JOO*E^Mg5V>8d0a9ybdgHVy;Mb2Fn{ONpyWj zL8>{?2+~18oPstn^)7K|#KxMT6Fkout +# @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; +}