Merge pull request #1445 in SNORT/snort3 from ~RUCOMBS/snort3:extra_issues to master

Squashed commit of the following:

commit fccab4fbc431abbc1857efffe6fa9affb9101100
Author: russ <rucombs@cisco.com>
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 <rucombs@cisco.com>
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 4cf2a97948f857d752bb56997956da644747cfa1..412c1d1473a017841af32fb9dac72b79900e0cbd 100644 (file)
--- 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<unsigned>& 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<unsigned>& 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 af5a3b5cd8fe9a345c255f88775c5b115a72a79c..5ff20c5f63d58ebd7ee43f8f0f0393c5fdd9029c 100644 (file)
--- 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<bool> 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 e1f5a588b6b05ecc2644b1e3fb0296344326da8d..e2ada1aa54187c8ed64d8db0685156d12fef07b4 100644 (file)
--- 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) &&