From: Russ Combs (rucombs) Date: Wed, 28 Nov 2018 01:17:45 +0000 (-0500) Subject: Merge pull request #1445 in SNORT/snort3 from ~RUCOMBS/snort3:extra_issues to master X-Git-Tag: 3.0.0-250~14 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1e70bb7063e319cefef61575085291809c748f73;p=thirdparty%2Fsnort3.git Merge pull request #1445 in SNORT/snort3 from ~RUCOMBS/snort3:extra_issues to master Squashed commit of the following: commit fccab4fbc431abbc1857efffe6fa9affb9101100 Author: russ Date: Tue Nov 27 18:08:58 2018 -0500 build: fix come cppcheck warnings: Comparison of a boolean expression with an integer other than 0 or 1 commit cac975509d4c9eed8feb212baf7b97d44ffcb767 Author: russ Date: Tue Nov 27 18:08:15 2018 -0500 build: reduce variable scope to address warnings --- diff --git a/src/loggers/alert_fast.cc b/src/loggers/alert_fast.cc index 4cf2a9794..412c1d147 100644 --- a/src/loggers/alert_fast.cc +++ b/src/loggers/alert_fast.cc @@ -159,6 +159,9 @@ public: void alert(Packet*, const char* msg, const Event&) override; +private: + void log_data(Packet*, const Event&); + private: string file; unsigned long limit; @@ -235,70 +238,75 @@ void FastLogger::alert(Packet* p, const char* msg, const Event& event) TextLog_Print(fast_log, "{%s} ", p->get_type()); LogIpAddrs(fast_log, p); - // log packet (p) if this is not an http request with one or more buffers - // because in that case packet data is also in http_headers or http_client_body - // only http provides buffers at present; http_raw_status is always - // available if a response was processed by http_inspect - bool log_pkt = true; - if ( packet || SnortConfig::output_app_data() ) { - TextLog_NewLine(fast_log); - Inspector* gadget = p->flow ? p->flow->gadget : nullptr; - const char** buffers = gadget ? gadget->get_api()->buffers : nullptr; + log_data(p, event); + } + TextLog_NewLine(fast_log); + TextLog_Flush(fast_log); +} - if ( buffers ) - { - InspectionBuffer buf; - const std::vector& idv = gadget->get_buf(HttpEnums::HTTP_BUFFER_RAW_STATUS, - p, buf) ? rsp_ids : req_ids; - bool rsp = (idv == rsp_ids); +// log packet (p) if this is not an http request with one or more buffers +// because in that case packet data is also in http_headers or http_client_body +// only http provides buffers at present; http_raw_status is always +// available if a response was processed by http_inspect +void FastLogger::log_data(Packet* p, const Event& event) +{ + bool log_pkt = true; - for ( auto id : idv ) - { + TextLog_NewLine(fast_log); + Inspector* gadget = p->flow ? p->flow->gadget : nullptr; + const char** buffers = gadget ? gadget->get_api()->buffers : nullptr; - if ( gadget->get_buf(id, p, buf) ) - LogNetData(fast_log, buf.data, buf.len, p, buffers[id-1]); + if ( buffers ) + { + InspectionBuffer buf; + const std::vector& idv = gadget->get_buf(HttpEnums::HTTP_BUFFER_RAW_STATUS, + p, buf) ? rsp_ids : req_ids; + bool rsp = (idv == rsp_ids); - log_pkt = rsp; - } - } - else if ( gadget ) + for ( auto id : idv ) { - InspectionBuffer buf; - - if ( gadget->get_buf(InspectionBuffer::IBT_KEY, p, buf) ) - LogNetData(fast_log, buf.data, buf.len, p); - if ( gadget->get_buf(InspectionBuffer::IBT_HEADER, p, buf) ) - LogNetData(fast_log, buf.data, buf.len, p); + if ( gadget->get_buf(id, p, buf) ) + LogNetData(fast_log, buf.data, buf.len, p, buffers[id-1]); - if ( gadget->get_buf(InspectionBuffer::IBT_BODY, p, buf) ) - LogNetData(fast_log, buf.data, buf.len, p); + log_pkt = rsp; } - if (p->has_ip()) - LogIPPkt(fast_log, p); + } + else if ( gadget ) + { + InspectionBuffer buf; - else if ( log_pkt and p->obfuscator ) - { - // FIXIT-P avoid string copy - std::string buf((const char*)p->data, p->dsize); + if ( gadget->get_buf(InspectionBuffer::IBT_KEY, p, buf) ) + LogNetData(fast_log, buf.data, buf.len, p); - for ( const auto& b : *p->obfuscator ) - buf.replace(b.offset, b.length, b.length, p->obfuscator->get_mask_char()); + if ( gadget->get_buf(InspectionBuffer::IBT_HEADER, p, buf) ) + LogNetData(fast_log, buf.data, buf.len, p); - LogNetData(fast_log, (const uint8_t*)buf.c_str(), p->dsize, p); - } - else if ( log_pkt ) - LogNetData(fast_log, p->data, p->dsize, p); + if ( gadget->get_buf(InspectionBuffer::IBT_BODY, p, buf) ) + LogNetData(fast_log, buf.data, buf.len, p); + } + if (p->has_ip()) + LogIPPkt(fast_log, p); - DataBuffer& buf = DetectionEngine::get_alt_buffer(p); + else if ( log_pkt and p->obfuscator ) + { + // FIXIT-P avoid string copy + std::string buf((const char*)p->data, p->dsize); + + for ( const auto& b : *p->obfuscator ) + buf.replace(b.offset, b.length, b.length, p->obfuscator->get_mask_char()); - if ( buf.len and event.sig_info->gid != 116 ) - LogNetData(fast_log, buf.data, buf.len, p, "alt"); + LogNetData(fast_log, (const uint8_t*)buf.c_str(), p->dsize, p); } - TextLog_NewLine(fast_log); - TextLog_Flush(fast_log); + else if ( log_pkt ) + LogNetData(fast_log, p->data, p->dsize, p); + + DataBuffer& buf = DetectionEngine::get_alt_buffer(p); + + if ( buf.len and event.sig_info->gid != 116 ) + LogNetData(fast_log, buf.data, buf.len, p, "alt"); } //------------------------------------------------------------------------- diff --git a/src/network_inspectors/packet_tracer/packet_tracer.cc b/src/network_inspectors/packet_tracer/packet_tracer.cc index af5a3b5cd..5ff20c5f6 100644 --- a/src/network_inspectors/packet_tracer/packet_tracer.cc +++ b/src/network_inspectors/packet_tracer/packet_tracer.cc @@ -576,26 +576,26 @@ TEST_CASE("pause", "[PacketTracer]") TestPacketTracer::pause(); TestPacketTracer::log("%s", test_str); - CHECK( TestPacketTracer::get_buff()[0] == '\0' ); - CHECK( TestPacketTracer::get_buff_len() == 0 ); + CHECK((TestPacketTracer::get_buff()[0] == '\0')); + CHECK((TestPacketTracer::get_buff_len() == 0)); TestPacketTracer::unpause(); TestPacketTracer::log("%s", test_str); - CHECK( TestPacketTracer::get_buff()[0] == '\0' ); - CHECK( TestPacketTracer::get_buff_len() == 0 ); + CHECK((TestPacketTracer::get_buff()[0] == '\0')); + CHECK((TestPacketTracer::get_buff_len() == 0)); TestPacketTracer::unpause(); TestPacketTracer::log("%s", test_str); - CHECK( TestPacketTracer::get_buff()[0] == '\0' ); - CHECK( TestPacketTracer::get_buff_len() == 0 ); + CHECK((TestPacketTracer::get_buff()[0] == '\0')); + CHECK((TestPacketTracer::get_buff_len() == 0)); TestPacketTracer::unpause(); TestPacketTracer::log("%s", test_str); CHECK( !strcmp(TestPacketTracer::get_buff(), test_str) ); - CHECK( TestPacketTracer::get_buff_len() == 10 ); + CHECK((TestPacketTracer::get_buff_len() == 10)); TestPacketTracer::thread_term(); } @@ -610,28 +610,28 @@ TEST_CASE("reasons", "[PacketTracer]") TestPacketTracer::register_verdict_reason(high, PacketTracer::PRIORITY_HIGH); // Init - CHECK( TestPacketTracer::get_reason() == VERDICT_REASON_NO_BLOCK ); + CHECK((TestPacketTracer::get_reason() == VERDICT_REASON_NO_BLOCK)); // Update TestPacketTracer::set_reason(low1); - CHECK( TestPacketTracer::get_reason() == low1 ); + CHECK((TestPacketTracer::get_reason() == low1)); // Don't update if already set TestPacketTracer::set_reason(VERDICT_REASON_NO_BLOCK); - CHECK( TestPacketTracer::get_reason() == low1 ); + CHECK((TestPacketTracer::get_reason() == low1)); TestPacketTracer::set_reason(low2); - CHECK( TestPacketTracer::get_reason() == low1 ); + CHECK((TestPacketTracer::get_reason() == low1)); // Always update for high priority TestPacketTracer::set_reason(high); - CHECK( TestPacketTracer::get_reason() == high ); + CHECK((TestPacketTracer::get_reason() == high)); // Dump resets reason TestPacketTracer::dump(nullptr); - CHECK( TestPacketTracer::get_reason() == VERDICT_REASON_NO_BLOCK ); + CHECK((TestPacketTracer::get_reason() == VERDICT_REASON_NO_BLOCK)); // Dump delivers reason to daq - CHECK( TestPacketTracer::get_dump_reason() == high ); + CHECK((TestPacketTracer::get_dump_reason() == high)); TestPacketTracer::thread_term(); } @@ -652,14 +652,14 @@ TEST_CASE("verbosity", "[PacketTracer]") std::string val = TestPacketTracer::get_buff(); std::string expected = "this should log\nthis should also log\n"; - CHECK( val == expected ); + CHECK((val == expected)); // reset mutes TestPacketTracer::dump(nullptr, 0); TestPacketTracer::log(mute_1, "this should log\n"); TestPacketTracer::log(mute_2, "this should also log\n"); val = TestPacketTracer::get_buff(); - CHECK( val == expected ); + CHECK((val == expected)); TestPacketTracer::thread_term(); } @@ -668,16 +668,16 @@ TEST_CASE("mute on inactive", "[PacketTracer]") { global_mutes.val = 0; - CHECK( TestPacketTracer::get_mute() == 0 ); - CHECK( TestPacketTracer::get_mute() == 1 ); - CHECK( TestPacketTracer::get_mute() == 2 ); + CHECK((TestPacketTracer::get_mute() == 0)); + CHECK((TestPacketTracer::get_mute() == 1)); + CHECK((TestPacketTracer::get_mute() == 2)); // activation mid-run TestPacketTracer::thread_init(); - CHECK( TestPacketTracer::get_mute() == 3 ); - CHECK( TestPacketTracer::get_mute() == 4 ); - CHECK( TestPacketTracer::get_mute() == 5 ); + CHECK((TestPacketTracer::get_mute() == 3)); + CHECK((TestPacketTracer::get_mute() == 4)); + CHECK((TestPacketTracer::get_mute() == 5)); std::vector expected = {false, false, false, false, false, false}; CHECK( TestPacketTracer::get_mutes() == expected ); diff --git a/src/network_inspectors/port_scan/ps_detect.cc b/src/network_inspectors/port_scan/ps_detect.cc index e1f5a588b..e2ada1aa5 100644 --- a/src/network_inspectors/port_scan/ps_detect.cc +++ b/src/network_inspectors/port_scan/ps_detect.cc @@ -557,7 +557,6 @@ void PortScan::ps_tracker_update_tcp(PS_PKT* ps_pkt, PS_TRACKER* scanner, PS_TRACKER* scanned) { Packet* p = (Packet*)ps_pkt->pkt; - uint32_t session_flags = 0x0; unsigned win = config->tcp_window; SfIp cleared; @@ -584,7 +583,7 @@ void PortScan::ps_tracker_update_tcp(PS_PKT* ps_pkt, PS_TRACKER* scanner, // this should be completely redone and port_scan should require stream_tcp if ( p->flow and (p->flow->ssn_state.session_flags & SSNFLAG_COUNTED_INITIALIZE) ) { - session_flags = p->flow->get_session_flags(); + uint32_t session_flags = p->flow->get_session_flags(); if ((session_flags & SSNFLAG_SEEN_CLIENT) && !(session_flags & SSNFLAG_SEEN_SERVER) &&