diff --git a/scripts/base/frameworks/notice/weird.bro b/scripts/base/frameworks/notice/weird.bro index 6c8ba14974..42bed543ee 100644 --- a/scripts/base/frameworks/notice/weird.bro +++ b/scripts/base/frameworks/notice/weird.bro @@ -106,6 +106,7 @@ export { ["baroque_SYN"] = ACTION_LOG, ["base64_illegal_encoding"] = ACTION_LOG, ["connection_originator_SYN_ack"] = ACTION_LOG_PER_ORIG, + ["contentline_size_exceeded"] = ACTION_LOG, ["corrupt_tcp_options"] = ACTION_LOG_PER_ORIG, ["crud_trailing_HTTP_request"] = ACTION_LOG, ["data_after_reset"] = ACTION_LOG, diff --git a/src/Sessions.cc b/src/Sessions.cc index 9e69a7b37a..a1e685a608 100644 --- a/src/Sessions.cc +++ b/src/Sessions.cc @@ -337,7 +337,7 @@ void NetSessions::DoNextPacket(double t, const Packet* pkt, const IP_Hdr* ip_hdr return; } - // for both of these it is safe to pass ip_hdr because the presence + // For both of these it is safe to pass ip_hdr because the presence // is guaranteed for the functions that pass data to us. int ip_hdr_len = ip_hdr->HdrLen(); if ( ip_hdr_len > len ) @@ -345,6 +345,7 @@ void NetSessions::DoNextPacket(double t, const Packet* pkt, const IP_Hdr* ip_hdr Weird("invalid_IP_header_size", ip_hdr, encapsulation); return; } + if ( ip_hdr_len > caplen ) { Weird("internally_truncated_header", ip_hdr, encapsulation); diff --git a/src/analyzer/protocol/finger/Finger.cc b/src/analyzer/protocol/finger/Finger.cc index a9818ff7af..e1be27e795 100644 --- a/src/analyzer/protocol/finger/Finger.cc +++ b/src/analyzer/protocol/finger/Finger.cc @@ -17,9 +17,9 @@ Finger_Analyzer::Finger_Analyzer(Connection* conn) : tcp::TCP_ApplicationAnalyzer("FINGER", conn) { did_deliver = 0; - content_line_orig = new tcp::ContentLine_Analyzer(conn, true); + content_line_orig = new tcp::ContentLine_Analyzer(conn, true, 1000); content_line_orig->SetIsNULSensitive(true); - content_line_resp = new tcp::ContentLine_Analyzer(conn, false); + content_line_resp = new tcp::ContentLine_Analyzer(conn, false, 1000); AddSupportAnalyzer(content_line_orig); AddSupportAnalyzer(content_line_resp); } diff --git a/src/analyzer/protocol/ident/Ident.cc b/src/analyzer/protocol/ident/Ident.cc index f668be921c..9601be7562 100644 --- a/src/analyzer/protocol/ident/Ident.cc +++ b/src/analyzer/protocol/ident/Ident.cc @@ -17,8 +17,8 @@ Ident_Analyzer::Ident_Analyzer(Connection* conn) { did_bad_reply = did_deliver = 0; - orig_ident = new tcp::ContentLine_Analyzer(conn, true); - resp_ident = new tcp::ContentLine_Analyzer(conn, false); + orig_ident = new tcp::ContentLine_Analyzer(conn, true, 1000); + resp_ident = new tcp::ContentLine_Analyzer(conn, false, 1000); orig_ident->SetIsNULSensitive(true); resp_ident->SetIsNULSensitive(true); diff --git a/src/analyzer/protocol/irc/IRC.cc b/src/analyzer/protocol/irc/IRC.cc index a26045f250..a69674eb50 100644 --- a/src/analyzer/protocol/irc/IRC.cc +++ b/src/analyzer/protocol/irc/IRC.cc @@ -21,9 +21,9 @@ IRC_Analyzer::IRC_Analyzer(Connection* conn) orig_zip_status = NO_ZIP; resp_zip_status = NO_ZIP; starttls = false; - cl_orig = new tcp::ContentLine_Analyzer(conn, true); + cl_orig = new tcp::ContentLine_Analyzer(conn, true, 1000); AddSupportAnalyzer(cl_orig); - cl_resp = new tcp::ContentLine_Analyzer(conn, false); + cl_resp = new tcp::ContentLine_Analyzer(conn, false, 1000); AddSupportAnalyzer(cl_resp); } diff --git a/src/analyzer/protocol/tcp/ContentLine.cc b/src/analyzer/protocol/tcp/ContentLine.cc index a830cc8a7d..7fc8085246 100644 --- a/src/analyzer/protocol/tcp/ContentLine.cc +++ b/src/analyzer/protocol/tcp/ContentLine.cc @@ -7,14 +7,14 @@ using namespace analyzer::tcp; -ContentLine_Analyzer::ContentLine_Analyzer(Connection* conn, bool orig) -: TCP_SupportAnalyzer("CONTENTLINE", conn, orig) +ContentLine_Analyzer::ContentLine_Analyzer(Connection* conn, bool orig, int max_line_length) +: TCP_SupportAnalyzer("CONTENTLINE", conn, orig), max_line_length(max_line_length) { InitState(); } -ContentLine_Analyzer::ContentLine_Analyzer(const char* name, Connection* conn, bool orig) -: TCP_SupportAnalyzer(name, conn, orig) +ContentLine_Analyzer::ContentLine_Analyzer(const char* name, Connection* conn, bool orig, int max_line_length) +: TCP_SupportAnalyzer(name, conn, orig), max_line_length(max_line_length) { InitState(); } @@ -229,6 +229,12 @@ int ContentLine_Analyzer::DoDeliverOnce(int len, const u_char* data) return seq_len; \ } + if ( offset >= max_line_length ) + { + Weird("contentline_size_exceeded"); + EMIT_LINE + } + switch ( c ) { case '\r': // Look ahead for '\n'. diff --git a/src/analyzer/protocol/tcp/ContentLine.h b/src/analyzer/protocol/tcp/ContentLine.h index 7a5a6b996e..5cb6d2f3d4 100644 --- a/src/analyzer/protocol/tcp/ContentLine.h +++ b/src/analyzer/protocol/tcp/ContentLine.h @@ -10,9 +10,12 @@ namespace analyzer { namespace tcp { #define CR_as_EOL 1 #define LF_as_EOL 2 +// Slightly smaller than 16MB so that the buffer is not unnecessarily resized to 32M. +#define DEFAULT_MAX_LINE_LENGTH 16 * 1024 * 1024 - 100 + class ContentLine_Analyzer : public TCP_SupportAnalyzer { public: - ContentLine_Analyzer(Connection* conn, bool orig); + ContentLine_Analyzer(Connection* conn, bool orig, int max_line_length=DEFAULT_MAX_LINE_LENGTH); ~ContentLine_Analyzer(); void SupressWeirds(bool enable) @@ -60,7 +63,7 @@ public: { return seq + length <= seq_to_skip; } protected: - ContentLine_Analyzer(const char* name, Connection* conn, bool orig); + ContentLine_Analyzer(const char* name, Connection* conn, bool orig, int max_line_length=DEFAULT_MAX_LINE_LENGTH); virtual void DeliverStream(int len, const u_char* data, bool is_orig); virtual void Undelivered(uint64 seq, int len, bool orig); @@ -80,6 +83,7 @@ protected: int offset; // where we are in buf int buf_len; // how big buf is, total unsigned int last_char; // last (non-option) character scanned + int max_line_length; // how large of a line to accumulate before emitting and raising a weird uint64_t seq; // last seq number uint64_t seq_to_skip; diff --git a/testing/btest/Baseline/scripts.base.protocols.irc.longline/weird.log b/testing/btest/Baseline/scripts.base.protocols.irc.longline/weird.log new file mode 100644 index 0000000000..b88f8724c5 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.irc.longline/weird.log @@ -0,0 +1,12 @@ +#separator \x09 +#set_separator , +#empty_field (empty) +#unset_field - +#path weird +#open 2017-11-03-19-17-18 +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer +#types time string addr port addr port string string bool string +1509735979.080381 CtPZjS20MLrsMUOJi2 127.0.0.1 50164 127.0.0.1 6667 contentline_size_exceeded - F bro +1509735979.080381 CtPZjS20MLrsMUOJi2 127.0.0.1 50164 127.0.0.1 6667 irc_line_size_exceeded - F bro +1509735981.241042 CtPZjS20MLrsMUOJi2 127.0.0.1 50164 127.0.0.1 6667 irc_invalid_command - F bro +#close 2017-11-03-19-17-18 diff --git a/testing/btest/Traces/contentline-irc-5k-line.pcap b/testing/btest/Traces/contentline-irc-5k-line.pcap new file mode 100644 index 0000000000..94c8815af2 Binary files /dev/null and b/testing/btest/Traces/contentline-irc-5k-line.pcap differ diff --git a/testing/btest/scripts/base/protocols/irc/longline.test b/testing/btest/scripts/base/protocols/irc/longline.test new file mode 100644 index 0000000000..0573494844 --- /dev/null +++ b/testing/btest/scripts/base/protocols/irc/longline.test @@ -0,0 +1,6 @@ +# This tests that an excessively long line is truncated by the contentline +# analyzer + +# @TEST-EXEC: bro -C -r $TRACES/contentline-irc-5k-line.pcap %INPUT +# @TEST-EXEC: btest-diff weird.log +