From: Ron Dempster (rdempste) Date: Tue, 31 Oct 2023 14:26:28 +0000 (+0000) Subject: Pull request #3935: Cppcheck X-Git-Tag: 3.1.74.0~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4758ecbe7c3898c79a88bbd01940b5dcc3f6ddc2;p=thirdparty%2Fsnort3.git Pull request #3935: Cppcheck Merge in SNORT/snort3 from ~RDEMPSTE/snort3:cppcheck to master Squashed commit of the following: commit e7663dd3be7fd74a91808f293de0222ea7a467ee Author: Ron Dempster (rdempste) Date: Thu Oct 19 11:46:13 2023 -0400 build: remove unused functions reported by cppcheck commit ce623c51d7bb6a034d09b3700db74f1ccf229bc5 Author: Ron Dempster (rdempste) Date: Wed Jul 19 12:55:56 2023 -0400 actions, detection, file_api, flow, stream: coverity fixes commit bed4872d9259d16a345e61a15f766505c18b0c30 Author: Ron Dempster (rdempste) Date: Fri Jun 30 13:57:44 2023 -0400 build: Address miscellaneous cppcheck warnings --- diff --git a/src/actions/act_react.cc b/src/actions/act_react.cc index cf89582a8..e52576470 100644 --- a/src/actions/act_react.cc +++ b/src/actions/act_react.cc @@ -101,7 +101,7 @@ class ReactData { public: - ReactData(const std::string& page) + explicit ReactData(const std::string& page) { if ( page.empty()) { @@ -136,7 +136,7 @@ private: class ReactActiveAction : public ActiveAction { public: - ReactActiveAction(ReactData* c) + explicit ReactActiveAction(ReactData* c) : ActiveAction( ActionPriority::AP_PROXY ), config(c) { } @@ -149,6 +149,7 @@ private: void ReactActiveAction::delayed_exec(Packet* p) { + // cppcheck-suppress unreadVariable Profile profile(reactPerfStats); if ( p->active->is_reset_candidate(p) ) diff --git a/src/actions/act_reject.cc b/src/actions/act_reject.cc index 79ff3ca1e..dbb1fe0dc 100644 --- a/src/actions/act_reject.cc +++ b/src/actions/act_reject.cc @@ -210,7 +210,7 @@ public: uint32_t get_data(); private: - uint32_t flags; + uint32_t flags = 0; }; bool RejectModule::begin(const char*, int, SnortConfig*) diff --git a/src/codecs/ip/cd_auth.cc b/src/codecs/ip/cd_auth.cc index a97c86914..93afcb842 100644 --- a/src/codecs/ip/cd_auth.cc +++ b/src/codecs/ip/cd_auth.cc @@ -66,9 +66,13 @@ struct AuthHdr { IpProtocol next; uint8_t len; + // cppcheck-suppress unusedStructMember uint16_t rsv; /* reserved */ + // cppcheck-suppress unusedStructMember uint32_t spi; /* Security Parameters Index */ + // cppcheck-suppress unusedStructMember uint32_t seq; /* Sequence Number */ + // cppcheck-suppress unusedStructMember uint32_t icv[1]; /* VARIABLE LENGTH!! -- specified by len field*/ }; diff --git a/src/codecs/ip/cd_gre.cc b/src/codecs/ip/cd_gre.cc index d8a3488fb..054a2071b 100644 --- a/src/codecs/ip/cd_gre.cc +++ b/src/codecs/ip/cd_gre.cc @@ -353,6 +353,6 @@ TEST_CASE ("Validate error check for raw_len greater than GRE_HEADER_LEN", "[cd_ Buffer buf(&t, size); Flow *flow = nullptr; - CHECK (grecodec.encode(&raw_in,raw_len,enc,buf,flow) == false); + CHECK (false == grecodec.encode(&raw_in,raw_len,enc,buf,flow)); } #endif diff --git a/src/codecs/ip/cd_icmp4.cc b/src/codecs/ip/cd_icmp4.cc index b8a698df2..334805ca2 100644 --- a/src/codecs/ip/cd_icmp4.cc +++ b/src/codecs/ip/cd_icmp4.cc @@ -565,9 +565,12 @@ namespace { struct IcmpHdr { + // cppcheck-suppress unusedStructMember uint8_t type; + // cppcheck-suppress unusedStructMember uint8_t code; uint16_t cksum; + // cppcheck-suppress unusedStructMember uint32_t unused; }; } // namespace diff --git a/src/codecs/ip/cd_icmp6.cc b/src/codecs/ip/cd_icmp6.cc index e68251355..1e90b4c66 100644 --- a/src/codecs/ip/cd_icmp6.cc +++ b/src/codecs/ip/cd_icmp6.cc @@ -341,9 +341,12 @@ namespace { struct IcmpHdr { + // cppcheck-suppress unusedStructMember uint8_t type; + // cppcheck-suppress unusedStructMember uint8_t code; uint16_t cksum; + // cppcheck-suppress unusedStructMember uint32_t unused; }; } // namespace diff --git a/src/codecs/ip/cd_mobility.cc b/src/codecs/ip/cd_mobility.cc index 292d9d535..2971aa3ac 100644 --- a/src/codecs/ip/cd_mobility.cc +++ b/src/codecs/ip/cd_mobility.cc @@ -46,8 +46,11 @@ struct MobileIPV6Header // RFC 6275 { IpProtocol payload_proto; uint8_t header_len; + // cppcheck-suppress unusedStructMember uint8_t mh_type; + // cppcheck-suppress unusedStructMember uint8_t reserved; + // cppcheck-suppress unusedStructMember uint16_t checksum; }; } // namespace diff --git a/src/codecs/ip/cd_pgm.cc b/src/codecs/ip/cd_pgm.cc index 2ffb5740a..2265d3279 100644 --- a/src/codecs/ip/cd_pgm.cc +++ b/src/codecs/ip/cd_pgm.cc @@ -66,18 +66,27 @@ struct PGM_NAK_OPT { uint8_t type; /* 02 = vuln */ uint8_t len; + // cppcheck-suppress unusedStructMember uint8_t res[2]; + // cppcheck-suppress unusedStructMember uint32_t seq[1]; /* could be many many more, but 1 is sufficient */ }; struct PGM_NAK { + // cppcheck-suppress unusedStructMember uint32_t seqnum; + // cppcheck-suppress unusedStructMember uint16_t afil1; + // cppcheck-suppress unusedStructMember uint16_t res1; + // cppcheck-suppress unusedStructMember uint32_t src; + // cppcheck-suppress unusedStructMember uint16_t afi2; + // cppcheck-suppress unusedStructMember uint16_t res2; + // cppcheck-suppress unusedStructMember uint32_t multi; PGM_NAK_OPT opt; }; diff --git a/src/codecs/link/cd_fabricpath.cc b/src/codecs/link/cd_fabricpath.cc index cb05559e5..d6bdf0619 100644 --- a/src/codecs/link/cd_fabricpath.cc +++ b/src/codecs/link/cd_fabricpath.cc @@ -63,6 +63,7 @@ struct FPathHdr uint8_t fpath_dst[6]; uint8_t fpath_src[6]; uint16_t fpath_type; + // cppcheck-suppress unusedStructMember uint16_t fptag_extra; /* 10-bit FTag + 6-bit TTL */ }; diff --git a/src/codecs/link/cd_pppoe.cc b/src/codecs/link/cd_pppoe.cc index 64da98ff2..fa11be896 100644 --- a/src/codecs/link/cd_pppoe.cc +++ b/src/codecs/link/cd_pppoe.cc @@ -38,8 +38,11 @@ enum class PppoepktType /* PPPoEHdr Header; eth::EtherHdr plus the PPPoE Header */ struct PPPoEHdr { + // cppcheck-suppress unusedStructMember uint8_t ver_type; /* pppoe version/type */ + // cppcheck-suppress unusedStructMember uint8_t code; /* pppoe code CODE_* */ + // cppcheck-suppress unusedStructMember uint16_t session; /* session id */ uint16_t length; /* payload length */ /* payload follows */ diff --git a/src/codecs/link/cd_vlan.cc b/src/codecs/link/cd_vlan.cc index 8036fdd1f..91aa648ea 100644 --- a/src/codecs/link/cd_vlan.cc +++ b/src/codecs/link/cd_vlan.cc @@ -83,8 +83,8 @@ bool VlanModule::set(const char*, Value& v, SnortConfig*) class VlanCodec : public Codec { public: - VlanCodec(const char* s) : Codec(CD_VLAN_NAME) - { tpids = s; } + VlanCodec(const char* s) : Codec(CD_VLAN_NAME), tpids(s) + { } void get_protocol_ids(std::vector& v) override; bool decode(const RawData&, CodecData&, DecodeData&) override; diff --git a/src/codecs/misc/cd_vxlan.cc b/src/codecs/misc/cd_vxlan.cc index e81f5ce2d..c187a8136 100644 --- a/src/codecs/misc/cd_vxlan.cc +++ b/src/codecs/misc/cd_vxlan.cc @@ -46,8 +46,10 @@ public: struct VXLANHdr { uint8_t flags; + // cppcheck-suppress unusedStructMember uint8_t reserved_1[3]; uint8_t vni[3]; //VXLAN network id + // cppcheck-suppress unusedStructMember uint8_t reserved_2; }; constexpr uint16_t VXLAN_MIN_HDR_LEN = 8; diff --git a/src/connectors/file_connector/file_connector_module.cc b/src/connectors/file_connector/file_connector_module.cc index ef3b65ad9..fb2a8cd35 100644 --- a/src/connectors/file_connector/file_connector_module.cc +++ b/src/connectors/file_connector/file_connector_module.cc @@ -59,16 +59,13 @@ extern THREAD_LOCAL ProfileStats file_connector_perfstats; FileConnectorModule::FileConnectorModule() : Module(FILE_CONNECTOR_NAME, FILE_CONNECTOR_HELP, file_connector_params, true) { - config = nullptr; config_set = new FileConnectorConfig::FileConnectorConfigSet; } FileConnectorModule::~FileConnectorModule() { - if ( config ) - delete config; - if ( config_set ) - delete config_set; + delete config; + delete config_set; } ProfileStats* FileConnectorModule::get_profile() const diff --git a/src/connectors/file_connector/file_connector_module.h b/src/connectors/file_connector/file_connector_module.h index d58417d56..020a563aa 100644 --- a/src/connectors/file_connector/file_connector_module.h +++ b/src/connectors/file_connector/file_connector_module.h @@ -50,7 +50,7 @@ public: private: FileConnectorConfig::FileConnectorConfigSet* config_set; - FileConnectorConfig* config; + FileConnectorConfig* config = nullptr; }; #endif diff --git a/src/connectors/file_connector/test/file_connector_test.cc b/src/connectors/file_connector/test/file_connector_test.cc index 5c6e48d86..930008a85 100644 --- a/src/connectors/file_connector/test/file_connector_test.cc +++ b/src/connectors/file_connector/test/file_connector_test.cc @@ -59,12 +59,10 @@ const char* get_instance_file(std::string& file, const char* name) FileConnectorModule::FileConnectorModule() : Module("FC", "FC Help", nullptr) -{ } +{ config_set = nullptr; } FileConnectorConfig::FileConnectorConfigSet* FileConnectorModule::get_and_clear_config() -{ - return new FileConnectorConfig::FileConnectorConfigSet; -} +{ return new FileConnectorConfig::FileConnectorConfigSet; } FileConnectorModule::~FileConnectorModule() = default; diff --git a/src/connectors/tcp_connector/tcp_connector.cc b/src/connectors/tcp_connector/tcp_connector.cc index 360cbd213..ff6797d20 100644 --- a/src/connectors/tcp_connector/tcp_connector.cc +++ b/src/connectors/tcp_connector/tcp_connector.cc @@ -69,11 +69,11 @@ TcpConnectorCommon::~TcpConnectorCommon() enum ReadDataOutcome { SUCCESS = 0, TRUNCATED, ERROR, CLOSED, PARTIAL, AGAIN }; -static ReadDataOutcome read_data(int sockfd, uint8_t *data, uint16_t length, ssize_t *read_offset) +static ReadDataOutcome read_data(int sockfd, uint8_t *data, uint16_t length, ssize_t& read_offset) { ssize_t bytes_read, offset; - offset = *read_offset; + offset = read_offset; bytes_read = recv(sockfd, data + offset, length - offset, 0); if (bytes_read == 0) { @@ -91,7 +91,7 @@ static ReadDataOutcome read_data(int sockfd, uint8_t *data, uint16_t length, ssi } return ERROR; } - *read_offset = offset + bytes_read; + read_offset = offset + bytes_read; if ((offset + bytes_read) < length) return PARTIAL; @@ -103,10 +103,10 @@ static ReadDataOutcome read_message_data(int sockfd, uint16_t length, uint8_t *d if ( length > 0 ) { ReadDataOutcome rval; - ssize_t offset = 0; do { - rval = read_data(sockfd, data, length, &offset); + ssize_t offset = 0; + rval = read_data(sockfd, data, length, offset); } while (rval == PARTIAL || rval == AGAIN); if (rval != SUCCESS) diff --git a/src/connectors/tcp_connector/tcp_connector.h b/src/connectors/tcp_connector/tcp_connector.h index 3782a12cd..882b4c801 100644 --- a/src/connectors/tcp_connector/tcp_connector.h +++ b/src/connectors/tcp_connector/tcp_connector.h @@ -37,7 +37,8 @@ class __attribute__((__packed__)) TcpConnectorMsgHdr { public: - TcpConnectorMsgHdr() = default; + TcpConnectorMsgHdr() : version(0), connector_msg_length(0) + { } TcpConnectorMsgHdr(uint32_t length) { version = TCP_FORMAT_VERSION; connector_msg_length = length; } diff --git a/src/connectors/tcp_connector/tcp_connector_module.cc b/src/connectors/tcp_connector/tcp_connector_module.cc index d7d3adc10..bc009c9fb 100644 --- a/src/connectors/tcp_connector/tcp_connector_module.cc +++ b/src/connectors/tcp_connector/tcp_connector_module.cc @@ -59,7 +59,6 @@ extern THREAD_LOCAL ProfileStats tcp_connector_perfstats; TcpConnectorModule::TcpConnectorModule() : Module(TCP_CONNECTOR_NAME, TCP_CONNECTOR_HELP, tcp_connector_params, true) { - config = nullptr; config_set = new TcpConnectorConfig::TcpConnectorConfigSet; } diff --git a/src/connectors/tcp_connector/tcp_connector_module.h b/src/connectors/tcp_connector/tcp_connector_module.h index b308503a4..2d7f34bf0 100644 --- a/src/connectors/tcp_connector/tcp_connector_module.h +++ b/src/connectors/tcp_connector/tcp_connector_module.h @@ -50,7 +50,7 @@ public: private: TcpConnectorConfig::TcpConnectorConfigSet* config_set; - TcpConnectorConfig* config; + TcpConnectorConfig* config = nullptr; }; #endif diff --git a/src/connectors/tcp_connector/test/tcp_connector_test.cc b/src/connectors/tcp_connector/test/tcp_connector_test.cc index 202c8ba2a..700601697 100644 --- a/src/connectors/tcp_connector/test/tcp_connector_test.cc +++ b/src/connectors/tcp_connector/test/tcp_connector_test.cc @@ -170,7 +170,7 @@ static void set_normal_status() TcpConnectorModule::TcpConnectorModule() : Module("TCPC", "TCPC Help", nullptr) -{ } +{ config_set = nullptr; } TcpConnectorConfig::TcpConnectorConfigSet* TcpConnectorModule::get_and_clear_config() { diff --git a/src/decompress/file_decomp_pdf.cc b/src/decompress/file_decomp_pdf.cc index 5b6a29894..69b093f7f 100644 --- a/src/decompress/file_decomp_pdf.cc +++ b/src/decompress/file_decomp_pdf.cc @@ -396,7 +396,7 @@ static inline fd_status_t Handle_State_DICT_OBJECT(fd_session_t* SessionPtr, uin and handles other diversion such as nested Dict objects. If the /Filter token doesn't exist then we don't fill the Filter_Spec_Buf[]. If in skip mode, no need to look for token. */ - char Filter_Tok[] = TOK_DICT_FILT; + static const char Filter_Tok[] = TOK_DICT_FILT; if ( (p->Sub_State == P_DICT_ACTIVE) && c == Filter_Tok[p->Elem_Index++] ) { diff --git a/src/decompress/file_olefile.cc b/src/decompress/file_olefile.cc index 594478f36..5c7bed68f 100644 --- a/src/decompress/file_olefile.cc +++ b/src/decompress/file_olefile.cc @@ -86,12 +86,12 @@ void OleFile :: walk_directory_list() name_buf = new uint8_t[32]; // The filename is UTF16 encoded and will be of the size 64 bytes. - dir_list->utf_state = new snort::UtfDecodeSession(); + snort::UtfDecodeSession utf_state; if (!header->get_byte_order()) - dir_list->utf_state->set_decode_utf_state_charset(CHARSET_UTF16LE); + utf_state.set_decode_utf_state_charset(CHARSET_UTF16LE); else - dir_list->utf_state->set_decode_utf_state_charset(CHARSET_UTF16BE); - dir_list->utf_state->decode_utf(buf, OLE_MAX_FILENAME_LEN_UTF16, name_buf, + utf_state.set_decode_utf_state_charset(CHARSET_UTF16BE); + utf_state.decode_utf(buf, OLE_MAX_FILENAME_LEN_UTF16, name_buf, OLE_MAX_FILENAME_ASCII, &bytes_copied); node->set_name(name_buf); @@ -129,7 +129,6 @@ void OleFile :: walk_directory_list() else dir_list->oleentry.insert({ file_name, node }); count++; - delete dir_list->utf_state; } // Reading the next sector of current_sector by referring the FAT list array. // A negative number suggests the end of directory entry array and there are @@ -561,14 +560,6 @@ int32_t cli_readn(const uint8_t*& fd, uint32_t& data_len, void* buff, int32_t co void OleFile :: decompression(const uint8_t* data, uint32_t& data_len, uint8_t*& local_vba_buffer, uint32_t& vba_buffer_offset) { - int16_t header; - bool flagCompressed; - unsigned char buffer[VBA_COMPRESSION_WINDOW]={ }; - uint16_t token; - unsigned int pos, shift, mask, distance; - uint8_t flag; - bool clean; - if (!data) return; @@ -579,11 +570,11 @@ void OleFile :: decompression(const uint8_t* data, uint32_t& data_len, uint8_t*& return; } - header = LETOHS_UNALIGNED(data + 1); + int16_t data_header = LETOHS_UNALIGNED(data + 1); - flagCompressed = header & 0x8000; + bool flagCompressed = 0 != (data_header & 0x8000); - if (((header >> 12) & 0x07) != 0b011) + if (((data_header >> 12) & 0x07) != 0b011) { VBA_DEBUG(vba_data_trace, DEFAULT_TRACE_OPTION_ID, TRACE_INFO_LEVEL, CURRENT_PACKET, "Invalid Chunk signature.\n"); @@ -592,35 +583,35 @@ void OleFile :: decompression(const uint8_t* data, uint32_t& data_len, uint8_t*& data += 3; data_len -= 3; + unsigned char buffer[VBA_COMPRESSION_WINDOW]={ }; if (flagCompressed == 0) { memcpy(&buffer, data, data_len); return; } - pos = 0; - clean = 1; + unsigned pos = 0; + bool clean = true; uint32_t size = data_len; + uint8_t flag; while (cli_readn(data, size, &flag, 1)) { - for (mask = 1; mask < 0x100; mask <<= 1) + for (unsigned mask = 1; mask < 0x100; mask <<= 1) { unsigned int winpos = pos % VBA_COMPRESSION_WINDOW; if (flag & mask) { - uint16_t len; - uint32_t srcpos; - + uint16_t token; if (!cli_readn(data, size, &token, 2)) return; - shift = 12 - (winpos > 0x10) - (winpos > 0x20) - (winpos > 0x40) - (winpos > + unsigned shift = 12 - (winpos > 0x10) - (winpos > 0x20) - (winpos > 0x40) - (winpos > 0x80) - (winpos > 0x100) - (winpos > 0x200) - (winpos > 0x400) - (winpos > 0x800); - len = (uint16_t)((token & ((1 << shift) - 1)) + 3); - distance = token >> shift; + uint16_t len = (uint16_t)((token & ((1 << shift) - 1)) + 3); + unsigned distance = token >> shift; - srcpos = pos - distance - 1; + uint32_t srcpos = pos - distance - 1; if ((((srcpos + len) % VBA_COMPRESSION_WINDOW) < winpos)and ((winpos + len) < VBA_COMPRESSION_WINDOW) and (((srcpos % VBA_COMPRESSION_WINDOW) + len) < VBA_COMPRESSION_WINDOW) and @@ -642,17 +633,18 @@ void OleFile :: decompression(const uint8_t* data, uint32_t& data_len, uint8_t*& { if ((pos != 0)and (winpos == 0) and clean) { + uint16_t token; if (cli_readn(data, size, &token, 2) != 2) { return; } - clean = 0; + clean = false; break; } if (cli_readn(data, size, &buffer[winpos], 1) == 1) pos++; } - clean = 1; + clean = true; } } diff --git a/src/decompress/file_olefile.h b/src/decompress/file_olefile.h index d1584221a..898c4b4fd 100644 --- a/src/decompress/file_olefile.h +++ b/src/decompress/file_olefile.h @@ -198,7 +198,7 @@ class DirectoryList { public: std::unordered_map oleentry; - snort::UtfDecodeSession* utf_state; + snort::UtfDecodeSession* utf_state = nullptr; bool is_file_exists(char* name); FileProperty* get_file_node(char* name); @@ -214,16 +214,12 @@ public: return mini_stream_sector; } - DirectoryList() - { - utf_state = nullptr; - mini_stream_sector = -1; - } + DirectoryList() = default; ~DirectoryList(); private: - int32_t mini_stream_sector; + int32_t mini_stream_sector = -1; }; class OleFile diff --git a/src/decompress/test/file_olefile_test.cc b/src/decompress/test/file_olefile_test.cc index c2a301639..ddc27be44 100644 --- a/src/decompress/test/file_olefile_test.cc +++ b/src/decompress/test/file_olefile_test.cc @@ -47,7 +47,6 @@ LiteralSearch* LiteralSearch::instantiate(LiteralSearch::Handle*, const uint8_t* void UtfDecodeSession::set_decode_utf_state_charset(CharsetCode, CharsetSrc) { } bool UtfDecodeSession::decode_utf(unsigned char const*, unsigned int, unsigned char*, unsigned int, int*) { return true; } -UtfDecodeSession::UtfDecodeSession() { } Packet* DetectionEngine::get_current_packet() { return nullptr; } void trace_vprintf(char const*, unsigned char, char const*, snort::Packet const*, char const*, va_list) { } uint8_t TraceApi::get_constraints_generation() { return 0; } diff --git a/src/detection/context_switcher.cc b/src/detection/context_switcher.cc index c4eb3478b..0da22a7e0 100644 --- a/src/detection/context_switcher.cc +++ b/src/detection/context_switcher.cc @@ -50,7 +50,7 @@ ContextSwitcher::~ContextSwitcher() { abort(); - for ( auto* p : contexts ) + for ( const auto* p : contexts ) delete p; } diff --git a/src/detection/detection_continuation.h b/src/detection/detection_continuation.h index 6ceb197f4..891b6c25f 100644 --- a/src/detection/detection_continuation.h +++ b/src/detection/detection_continuation.h @@ -245,16 +245,16 @@ bool Continuation::State::eval(snort::Packet& p) for (uint8_t i = 0; i < NUM_IPS_OPTIONS_VARS; ++i) snort::SetVarValueByIndex(byte_extract_vars[i], i); - const detection_option_tree_node_t* node = root.children[0]; + const detection_option_tree_node_t* root_node = root.children[0]; if (opt_parent) { - for (int i = 0; i < node->num_children; ++i) - result += detection_option_node_evaluate(node->children[i], data, cursor); + for (int i = 0; i < root_node->num_children; ++i) + result += detection_option_node_evaluate(root_node->children[i], data, cursor); } else { - result = detection_option_node_evaluate(node, data, cursor); + result = detection_option_node_evaluate(root_node, data, cursor); } clear_trace_cursor_info(); diff --git a/src/detection/detection_engine.cc b/src/detection/detection_engine.cc index 4f5c3b916..25f7e3341 100644 --- a/src/detection/detection_engine.cc +++ b/src/detection/detection_engine.cc @@ -102,6 +102,8 @@ void DetectionEngine::thread_term() delete offloader; } +// Not sure why cppcheck doesn't think context is initialized +// cppcheck-suppress uninitMemberVar DetectionEngine::DetectionEngine() { context = Analyzer::get_switcher()->interrupt(); @@ -459,6 +461,7 @@ bool DetectionEngine::offload(Packet* p) if ( p->flow ? p->flow->context_chain.front() : sw->non_flow_chain.front() ) { + // cppcheck-suppress unreadVariable Profile profile(mpsePerfStats); p->context->searches.search_sync(); sw->suspend(); @@ -508,6 +511,7 @@ void DetectionEngine::onload(Flow* flow) void DetectionEngine::onload() { + // cppcheck-suppress unreadVariable Profile profile(mpsePerfStats); Packet* p; @@ -519,7 +523,7 @@ void DetectionEngine::onload() p->clear_offloaded(); - IpsContextChain& chain = p->flow ? p->flow->context_chain : + const IpsContextChain& chain = p->flow ? p->flow->context_chain : Analyzer::get_switcher()->non_flow_chain; resume_ready_suspends(chain); @@ -577,7 +581,9 @@ void DetectionEngine::wait_for_context() do { onload(); - } while ( !sw->idle_count() ); + } + // cppcheck-suppress knownConditionTrueFalse + while ( !sw->idle_count() ); } } @@ -744,6 +750,7 @@ static int log_events(void* event, void* user) */ int DetectionEngine::log_events(Packet* p) { + // cppcheck-suppress unreadVariable Profile profile(eventqPerfStats); SF_EVENTQ* pq = p->context->equeue; sfeventq_action(pq, ::log_events, (void*)p); diff --git a/src/detection/detection_options.cc b/src/detection/detection_options.cc index eaa805319..9b8ebdabe 100644 --- a/src/detection/detection_options.cc +++ b/src/detection/detection_options.cc @@ -405,7 +405,6 @@ int detection_option_node_evaluate( bool continue_loop = true; int loop_count = 0; - char tmp_noalert_flag = 0; int result = 0; uint32_t tmp_byte_extract_vars[NUM_IPS_OPTIONS_VARS]; IpsOption* buf_selector = eval_data.buf_selector; @@ -425,14 +424,10 @@ int detection_option_node_evaluate( { const auto& sig_info = node->otn->sigInfo; - for ( const auto& svc : sig_info.services ) - { - if ( snort_protocol_id == svc.snort_protocol_id ) - { - check_ports = 0; - break; // out of for - } - } + if ( std::any_of(sig_info.services.cbegin(), sig_info.services.cend(), + [snort_protocol_id] (const SignatureServiceInfo& svc) + { return snort_protocol_id == svc.snort_protocol_id; }) ) + check_ports = 0; if ( !sig_info.services.empty() and check_ports ) { @@ -541,7 +536,7 @@ int detection_option_node_evaluate( Continuation::postpone(cursor, *node, eval_data); return result; } - else if ( rval == (int)IpsOption::FAILED_BIT ) + if ( rval == (int)IpsOption::FAILED_BIT ) { debug_log(detection_trace, TRACE_RULE_EVAL, p, "failed bit\n"); eval_data.flowbit_failed = 1; @@ -550,11 +545,12 @@ int detection_option_node_evaluate( state.last_check.result = result; return 0; } - else if ( rval == (int)IpsOption::NO_ALERT ) + + // Cache the current flowbit_noalert flag, and set it + // so nodes below this don't alert. + char tmp_noalert_flag = eval_data.flowbit_noalert; + if ( rval == (int)IpsOption::NO_ALERT ) { - // Cache the current flowbit_noalert flag, and set it - // so nodes below this don't alert. - tmp_noalert_flag = eval_data.flowbit_noalert; eval_data.flowbit_noalert = 1; debug_log(detection_trace, TRACE_RULE_EVAL, p, "flowbit no alert\n"); } @@ -579,6 +575,8 @@ int detection_option_node_evaluate( if ( PacketLatency::fastpath() ) { + // Reset the flowbit_noalert flag in eval data + eval_data.flowbit_noalert = tmp_noalert_flag; profile.stop(result != (int)IpsOption::NO_MATCH); state.last_check.result = result; return result; @@ -695,11 +693,8 @@ int detection_option_node_evaluate( } } - if ( rval == (int)IpsOption::NO_ALERT ) - { - // Reset the flowbit_noalert flag in eval data - eval_data.flowbit_noalert = tmp_noalert_flag; - } + // Reset the flowbit_noalert flag in eval data + eval_data.flowbit_noalert = tmp_noalert_flag; if ( continue_loop && rval == (int)IpsOption::MATCH && node->relative_children ) { diff --git a/src/detection/fp_config.cc b/src/detection/fp_config.cc index 5eee82735..f7c182eda 100644 --- a/src/detection/fp_config.cc +++ b/src/detection/fp_config.cc @@ -62,7 +62,7 @@ bool FastPatternConfig::set_search_method(const char* method) return true; } -const char* FastPatternConfig::get_search_method() +const char* FastPatternConfig::get_search_method() const { if ( !search_api ) return nullptr; diff --git a/src/detection/fp_config.h b/src/detection/fp_config.h index 5073afb97..3f7cc0dcd 100644 --- a/src/detection/fp_config.h +++ b/src/detection/fp_config.h @@ -125,7 +125,7 @@ public: { return rule_db_dir; } bool set_search_method(const char*); - const char* get_search_method(); + const char* get_search_method() const; bool set_offload_search_method(const char*); void set_max_pattern_len(unsigned); diff --git a/src/detection/fp_create.cc b/src/detection/fp_create.cc index 7cbccf92c..94af06a67 100644 --- a/src/detection/fp_create.cc +++ b/src/detection/fp_create.cc @@ -145,8 +145,8 @@ static bool new_sig(int num_children, detection_option_tree_node_t** nodes, OptT continue; OptTreeNode* cotn = (OptTreeNode*)child->option_data; - SigInfo& csi = cotn->sigInfo; - SigInfo& osi = otn->sigInfo; + const SigInfo& csi = cotn->sigInfo; + const SigInfo& osi = otn->sigInfo; if ( csi.gid == osi.gid and csi.sid == osi.sid and csi.rev == osi.rev ) return false; @@ -1360,9 +1360,9 @@ static void fpPrintServiceRuleMaps(SnortConfig* sc) fpPrintServiceRuleMapTable(sc->srmmTable->to_cli, "to client"); } -static void fp_print_service_rules(SnortConfig* sc, GHash* cli, GHash* srv) +static void fp_print_service_rules(SnortConfig* sc, GHash* to_srv, GHash* to_cli) { - if ( !cli->get_count() and !srv->get_count() ) + if ( !to_srv->get_count() and !to_cli->get_count() ) return; LogLabel("service rule counts to-srv to-cli"); @@ -1372,8 +1372,8 @@ static void fp_print_service_rules(SnortConfig* sc, GHash* cli, GHash* srv) while ( const char* svc = sc->proto_ref->get_name_sorted(idx++) ) { - SF_LIST* clist = (SF_LIST*)cli->find(svc); - SF_LIST* slist = (SF_LIST*)srv->find(svc); + SF_LIST* clist = (SF_LIST*)to_srv->find(svc); + SF_LIST* slist = (SF_LIST*)to_cli->find(svc); if ( !clist and !slist ) continue; diff --git a/src/detection/fp_detect.cc b/src/detection/fp_detect.cc index b75851654..d0c0df7ee 100644 --- a/src/detection/fp_detect.cc +++ b/src/detection/fp_detect.cc @@ -717,11 +717,15 @@ static inline int fpFinalSelectEvent(OtnxMatchData* omd, Packet* p) class MpseStash { public: + // for some reason cppcheck does not understand that all members are used in MpseStash::process struct MatchData { + // cppcheck-suppress unusedStructMember void* user; void* tree; + // cppcheck-suppress unusedStructMember void* list; + // cppcheck-suppress unusedStructMember int index; }; @@ -781,6 +785,7 @@ bool MpseStash::push(void* user, void* tree, int index, void* context, void* lis if ( !checker and qmax == queue.size() and is_packet_thread() ) { + // cppcheck-suppress unreadVariable Profile rule_profile(rulePerfStats); process((IpsContext*)context, queue); } @@ -1282,6 +1287,7 @@ static void fpEvalPacket(Packet* p, FPTask task) void fp_partial(Packet* p) { + // cppcheck-suppress unreadVariable Profile mpse_profile(mpsePerfStats); IpsContext* c = p->context; init_match_info(c); @@ -1299,10 +1305,12 @@ void fp_complete(Packet* p, bool search) if ( search ) { + // cppcheck-suppress unreadVariable Profile mpse_profile(mpsePerfStats); c->searches.search_sync(); } { + // cppcheck-suppress unreadVariable Profile rule_profile(rulePerfStats); if (p->flow && p->flow->ips_cont) @@ -1335,10 +1343,12 @@ static void fp_immediate(Packet* p) IpsContext* c = p->context; MpseStash* stash = c->stash; { + // cppcheck-suppress unreadVariable Profile mpse_profile(mpsePerfStats); c->searches.search_sync(); } { + // cppcheck-suppress unreadVariable Profile rule_profile(rulePerfStats); stash->process(c); c->searches.items.clear(); @@ -1349,11 +1359,13 @@ static void fp_immediate(MpseGroup* mpg, Packet* p, const uint8_t* buf, unsigned { MpseStash* stash = p->context->stash; { + // cppcheck-suppress unreadVariable Profile mpse_profile(mpsePerfStats); int start_state = 0; mpg->get_normal_mpse()->search(buf, len, rule_tree_queue, p->context, &start_state); } { + // cppcheck-suppress unreadVariable Profile rule_profile(rulePerfStats); stash->process(p->context); } @@ -1382,6 +1394,7 @@ static inline int fp_do_actions(OtnxMatchData* omd, Packet* p) void fp_eval_service_group(Packet* p, SnortProtocolId snort_protocol_id) { + // cppcheck-suppress unreadVariable Profile mpse_profile(mpsePerfStats); RuleGroup* svc = p->context->conf->sopgTable->get_port_group(true, snort_protocol_id); @@ -1404,6 +1417,7 @@ void fp_eval_service_group(Packet* p, SnortProtocolId snort_protocol_id) MpseStash* stash = c->stash; c->searches.search_sync(); { + // cppcheck-suppress unreadVariable Profile rule_profile(rulePerfStats); stash->process(c); diff --git a/src/detection/fp_utils.cc b/src/detection/fp_utils.cc index 066f310c8..13f5c6861 100644 --- a/src/detection/fp_utils.cc +++ b/src/detection/fp_utils.cc @@ -743,42 +743,42 @@ TEST_CASE("pmd_no_options", "[PatternMatchData]") { PatternMatchData pmd = { }; set_pmd(pmd, 0x0, "foo"); - CHECK(pmd.can_be_fp()); + CHECK(true == pmd.can_be_fp()); } TEST_CASE("pmd_negated", "[PatternMatchData]") { PatternMatchData pmd = { }; set_pmd(pmd, 0x1, "foo"); - CHECK(!pmd.can_be_fp()); + CHECK(false == pmd.can_be_fp()); } TEST_CASE("pmd_no_case", "[PatternMatchData]") { PatternMatchData pmd = { }; set_pmd(pmd, 0x2, "foo"); - CHECK(pmd.can_be_fp()); + CHECK(true == pmd.can_be_fp()); } TEST_CASE("pmd_relative", "[PatternMatchData]") { PatternMatchData pmd = { }; set_pmd(pmd, 0x4, "foo"); - CHECK(pmd.can_be_fp()); + CHECK(true == pmd.can_be_fp()); } TEST_CASE("pmd_negated_no_case", "[PatternMatchData]") { PatternMatchData pmd = { }; set_pmd(pmd, 0x3, "foo"); - CHECK(pmd.can_be_fp()); + CHECK(true == pmd.can_be_fp()); } TEST_CASE("pmd_negated_relative", "[PatternMatchData]") { PatternMatchData pmd = { }; set_pmd(pmd, 0x5, "foo"); - CHECK(!pmd.can_be_fp()); + CHECK(false == pmd.can_be_fp()); } TEST_CASE("pmd_negated_no_case_offset", "[PatternMatchData]") @@ -786,7 +786,7 @@ TEST_CASE("pmd_negated_no_case_offset", "[PatternMatchData]") PatternMatchData pmd = { }; set_pmd(pmd, 0x1, "foo"); pmd.offset = 3; - CHECK(!pmd.can_be_fp()); + CHECK(false == pmd.can_be_fp()); } TEST_CASE("pmd_negated_no_case_depth", "[PatternMatchData]") @@ -794,7 +794,7 @@ TEST_CASE("pmd_negated_no_case_depth", "[PatternMatchData]") PatternMatchData pmd = { }; set_pmd(pmd, 0x3, "foo"); pmd.depth = 1; - CHECK(!pmd.can_be_fp()); + CHECK(false == pmd.can_be_fp()); } TEST_CASE("fp_simple", "[FastPatternSelect]") @@ -803,10 +803,10 @@ TEST_CASE("fp_simple", "[FastPatternSelect]") PatternMatchData pmd = { }; set_pmd(pmd, 0x0, "foo"); FpSelector left(CAT_SET_RAW, nullptr, &pmd); - CHECK(left.is_better_than(test, false, RULE_WO_DIR)); + CHECK(true == left.is_better_than(test, false, RULE_WO_DIR)); test.size = 1; - CHECK(left.is_better_than(test, false, RULE_WO_DIR)); + CHECK(true == left.is_better_than(test, false, RULE_WO_DIR)); } TEST_CASE("fp_negated", "[FastPatternSelect]") @@ -819,8 +819,8 @@ TEST_CASE("fp_negated", "[FastPatternSelect]") set_pmd(p1, 0x1, "foo"); FpSelector s1(CAT_SET_RAW, nullptr, &p1); - CHECK(s0.is_better_than(s1, false, RULE_WO_DIR)); - CHECK(!s1.is_better_than(s0, false, RULE_WO_DIR)); + CHECK(true == s0.is_better_than(s1, false, RULE_WO_DIR)); + CHECK(false == s1.is_better_than(s0, false, RULE_WO_DIR)); } TEST_CASE("fp_cat1", "[FastPatternSelect]") @@ -833,7 +833,7 @@ TEST_CASE("fp_cat1", "[FastPatternSelect]") set_pmd(p1, 0x0, "short"); FpSelector s1(CAT_SET_FAST_PATTERN, nullptr, &p1); - CHECK(s0.is_better_than(s1, true, RULE_WO_DIR)); + CHECK(true == s0.is_better_than(s1, true, RULE_WO_DIR)); } TEST_CASE("fp_cat2", "[FastPatternSelect]") @@ -846,8 +846,8 @@ TEST_CASE("fp_cat2", "[FastPatternSelect]") set_pmd(p1, 0x0, "foo"); FpSelector s1(CAT_SET_FAST_PATTERN, nullptr, &p1); - CHECK(!s0.is_better_than(s1, false, RULE_WO_DIR)); - CHECK(!s1.is_better_than(s0, false, RULE_WO_DIR)); + CHECK(false == s0.is_better_than(s1, false, RULE_WO_DIR)); + CHECK(false == s1.is_better_than(s0, false, RULE_WO_DIR)); } TEST_CASE("fp_cat3", "[FastPatternSelect]") @@ -860,7 +860,7 @@ TEST_CASE("fp_cat3", "[FastPatternSelect]") set_pmd(p1, 0x0, "foo"); FpSelector s1(CAT_SET_FAST_PATTERN, nullptr, &p1); - CHECK(!s0.is_better_than(s1, true, RULE_WO_DIR)); + CHECK(false == s0.is_better_than(s1, true, RULE_WO_DIR)); } TEST_CASE("fp_size", "[FastPatternSelect]") @@ -873,7 +873,7 @@ TEST_CASE("fp_size", "[FastPatternSelect]") set_pmd(p1, 0x0, "short"); FpSelector s1(CAT_SET_FAST_PATTERN, nullptr, &p1); - CHECK(s0.is_better_than(s1, false, RULE_WO_DIR)); + CHECK(true == s0.is_better_than(s1, false, RULE_WO_DIR)); } TEST_CASE("fp_pkt_key_port", "[FastPatternSelect]") @@ -886,7 +886,7 @@ TEST_CASE("fp_pkt_key_port", "[FastPatternSelect]") set_pmd(p1, 0x0, "longer"); FpSelector s1(CAT_SET_FAST_PATTERN, nullptr, &p1); - CHECK(!s0.is_better_than(s1, false, RULE_WO_DIR)); + CHECK(false == s0.is_better_than(s1, false, RULE_WO_DIR)); } TEST_CASE("fp_pkt_key_port_user", "[FastPatternSelect]") @@ -899,7 +899,7 @@ TEST_CASE("fp_pkt_key_port_user", "[FastPatternSelect]") set_pmd(p1, 0x0, "longer"); FpSelector s1(CAT_SET_FAST_PATTERN, nullptr, &p1); - CHECK(s0.is_better_than(s1, false, RULE_WO_DIR)); + CHECK(true == s0.is_better_than(s1, false, RULE_WO_DIR)); } TEST_CASE("fp_pkt_key_port_user_user", "[FastPatternSelect]") @@ -912,7 +912,7 @@ TEST_CASE("fp_pkt_key_port_user_user", "[FastPatternSelect]") set_pmd(p1, 0x10, "short"); FpSelector s1(CAT_SET_FAST_PATTERN, nullptr, &p1); - CHECK(!s0.is_better_than(s1, false, RULE_WO_DIR)); + CHECK(false == s0.is_better_than(s1, false, RULE_WO_DIR)); } TEST_CASE("fp_pkt_key_port_user_user2", "[FastPatternSelect]") @@ -925,7 +925,7 @@ TEST_CASE("fp_pkt_key_port_user_user2", "[FastPatternSelect]") set_pmd(p1, 0x10, "short"); FpSelector s1(CAT_SET_FAST_PATTERN, nullptr, &p1); - CHECK(!s0.is_better_than(s1, false, RULE_WO_DIR)); + CHECK(false == s0.is_better_than(s1, false, RULE_WO_DIR)); } TEST_CASE("fp_pkt_key_srvc_1", "[FastPatternSelect]") @@ -938,7 +938,7 @@ TEST_CASE("fp_pkt_key_srvc_1", "[FastPatternSelect]") set_pmd(p1, 0x0, "longer"); FpSelector s1(CAT_SET_FAST_PATTERN, nullptr, &p1); - CHECK(s1.is_better_than(s0, true, RULE_WO_DIR)); + CHECK(true == s1.is_better_than(s0, true, RULE_WO_DIR)); } TEST_CASE("fp_pkt_key_srvc_2", "[FastPatternSelect]") @@ -951,7 +951,7 @@ TEST_CASE("fp_pkt_key_srvc_2", "[FastPatternSelect]") set_pmd(p1, 0x0, "short"); FpSelector s1(CAT_SET_FAST_PATTERN, nullptr, &p1); - CHECK(s0.is_better_than(s1, true, RULE_WO_DIR)); + CHECK(true == s0.is_better_than(s1, true, RULE_WO_DIR)); } TEST_CASE("fp_pkt_key_srvc_rsp", "[FastPatternSelect]") @@ -964,8 +964,8 @@ TEST_CASE("fp_pkt_key_srvc_rsp", "[FastPatternSelect]") set_pmd(p1, 0x0, "longer"); FpSelector s1(CAT_SET_FAST_PATTERN, nullptr, &p1); - CHECK(!s0.is_better_than(s1, true, RULE_FROM_SERVER)); - CHECK(s1.is_better_than(s0, true, RULE_FROM_SERVER)); + CHECK(false == s0.is_better_than(s1, true, RULE_FROM_SERVER)); + CHECK(true == s1.is_better_than(s0, true, RULE_FROM_SERVER)); } #endif diff --git a/src/detection/pattern_match_data.h b/src/detection/pattern_match_data.h index 5b08e7063..a2b026880 100644 --- a/src/detection/pattern_match_data.h +++ b/src/detection/pattern_match_data.h @@ -144,12 +144,12 @@ inline bool PatternMatchData::can_be_fp() const inline bool PatternMatchData::has_alpha() const { - unsigned offset = fp_offset ? fp_offset : 0; - unsigned length = fp_length ? fp_length : pattern_size; + unsigned tmp_offset = static_cast(fp_offset); + unsigned tmp_length = fp_length ? fp_length : pattern_size; - for ( unsigned idx = 0; idx < length; ++idx ) + for ( unsigned idx = 0; idx < tmp_length; ++idx ) { - if ( isalpha(pattern_buf[offset + idx]) ) + if ( isalpha(pattern_buf[tmp_offset + idx]) ) return true; } return false; diff --git a/src/detection/regex_offload.cc b/src/detection/regex_offload.cc index 75b002fe7..7db47be84 100644 --- a/src/detection/regex_offload.cc +++ b/src/detection/regex_offload.cc @@ -90,7 +90,7 @@ RegexOffload::~RegexOffload() { assert(busy.empty()); - for ( auto* req : idle ) + for ( const auto* req : idle ) delete req; } @@ -99,14 +99,9 @@ void RegexOffload::stop() assert(busy.empty()); } -bool RegexOffload::on_hold(Flow* f) const +bool RegexOffload::on_hold(const Flow* f) const { - for ( auto* req : busy ) - { - if ( req->packet->flow == f ) - return true; - } - return false; + return std::any_of(busy.cbegin(), busy.cend(), [f](const RegexRequest* req){ return req->packet->flow == f; }); } //-------------------------------------------------------------------------- @@ -117,6 +112,7 @@ MpseRegexOffload::MpseRegexOffload(unsigned max) : RegexOffload(max) { } void MpseRegexOffload::put(Packet* p) { + // cppcheck-suppress unreadVariable Profile profile(mpsePerfStats); assert(p); @@ -137,6 +133,7 @@ void MpseRegexOffload::put(Packet* p) bool MpseRegexOffload::get(Packet*& p) { + // cppcheck-suppress unreadVariable Profile profile(mpsePerfStats); assert(!busy.empty()); @@ -211,6 +208,7 @@ void ThreadRegexOffload::stop() void ThreadRegexOffload::put(Packet* p) { + // cppcheck-suppress unreadVariable Profile profile(mpsePerfStats); assert(p); diff --git a/src/detection/regex_offload.h b/src/detection/regex_offload.h index 8f2533ca3..ced7e3ba5 100644 --- a/src/detection/regex_offload.h +++ b/src/detection/regex_offload.h @@ -57,7 +57,7 @@ public: unsigned count() const { return busy.size(); } - bool on_hold(snort::Flow*) const; + bool on_hold(const snort::Flow*) const; protected: RegexOffload(unsigned max); diff --git a/src/detection/service_map.cc b/src/detection/service_map.cc index 85d70f762..2a95148cf 100644 --- a/src/detection/service_map.cc +++ b/src/detection/service_map.cc @@ -228,7 +228,7 @@ sopg_table_t::sopg_table_t(unsigned n) RuleGroup* sopg_table_t::get_port_group(bool c2s, SnortProtocolId snort_protocol_id) { - RuleGroupVector& v = c2s ? to_srv : to_cli; + const RuleGroupVector& v = c2s ? to_srv : to_cli; if ( snort_protocol_id >= v.size() ) return nullptr; diff --git a/src/detection/signature.cc b/src/detection/signature.cc index 4c5ddb3ba..c50a3243d 100644 --- a/src/detection/signature.cc +++ b/src/detection/signature.cc @@ -81,11 +81,11 @@ static const ReferenceSystem* reference_system_lookup(SnortConfig* sc, const std void add_reference( SnortConfig* sc, OptTreeNode* otn, const std::string& system, const std::string& id) { + assert(sc and otn and !system.empty() and !id.empty()); + if ( !sc->alert_refs() ) return; - assert(sc and otn and !system.empty() and !id.empty()); - const ReferenceSystem* sys = reference_system_lookup(sc, system); if ( !sys ) @@ -331,7 +331,7 @@ static void dump_services(JsonStream& json, const SigInfo& si) json.close_array(); } -static void dump_bits(JsonStream& json, const char* key, std::vector& bits) +static void dump_bits(JsonStream& json, const char* key, const std::vector& bits) { if ( bits.empty() ) return; diff --git a/src/detection/tag.cc b/src/detection/tag.cc index 23b4d8ed2..7cae0c89f 100644 --- a/src/detection/tag.cc +++ b/src/detection/tag.cc @@ -183,7 +183,7 @@ static THREAD_LOCAL TagSessionCache* ssn_tag_cache = nullptr; * * @returns number of bytes needed */ -static inline unsigned int memory_per_node(XHash* hash) +static inline unsigned int memory_per_node(const XHash* hash) { if ( hash == ssn_tag_cache ) return sizeof(tTagFlowKey) + sizeof(HashNode) + sizeof(TagNode); diff --git a/src/detection/treenodes.h b/src/detection/treenodes.h index 3c1fd917d..17f2b3969 100644 --- a/src/detection/treenodes.h +++ b/src/detection/treenodes.h @@ -203,7 +203,7 @@ struct OptTreeNode SnortProtocolId snort_protocol_id = 0; // Added for integrity checks during rule parsing. unsigned short proto_node_num = 0; uint16_t longestPatternLen = 0; - IpsPolicy::Enable enable; + IpsPolicy::Enable enable = IpsPolicy::Enable::DISABLED; Flag flags = 0; enum SectionDir { SECT_TO_SRV = 0, SECT_TO_CLIENT, SECT_DIR__MAX }; diff --git a/src/dump_config/config_data.cc b/src/dump_config/config_data.cc index 06090f055..df88ddf0b 100644 --- a/src/dump_config/config_data.cc +++ b/src/dump_config/config_data.cc @@ -64,12 +64,9 @@ TreeConfigNode::TreeConfigNode(BaseConfigNode* parent_node, BaseConfigNode* TreeConfigNode::get_node(const std::string& name) { - for ( auto node : children ) - { - if ( node->get_name() == name ) - return node; - } - return nullptr; + auto it = std::find_if(children.cbegin(), children.cend(), + [name](BaseConfigNode* node){ return node->get_name() == name; }); + return it != children.cend() ? *it : nullptr; } ValueConfigNode::ValueConfigNode(BaseConfigNode* parent_node, const Value& val, @@ -311,7 +308,7 @@ TEST_CASE("value_config_node", "[ValueConfigNode]") SECTION("get_value") { CHECK(value_node_str->get_value()->get_origin_string() == "test_str"); - CHECK(value_node_bool->get_value()->get_bool() == true); + CHECK(true == value_node_bool->get_value()->get_bool()); CHECK(value_node_multi->get_value()->get_origin_string() == "test2 test3"); CHECK(value_node_custom_name->get_value()->get_origin_string() == "test_str_custom"); } @@ -337,7 +334,7 @@ TEST_CASE("value_config_node", "[ValueConfigNode]") SECTION("get_value_after_update") { CHECK(value_node_str->get_value()->get_origin_string() == "new_value"); - CHECK(value_node_bool->get_value()->get_bool() == false); + CHECK(false == value_node_bool->get_value()->get_bool()); CHECK(value_node_multi->get_value()->get_origin_string() == "test1 test2 test3"); CHECK(value_node_custom_name->get_value()->get_origin_string() == "new_custom_value"); } diff --git a/src/file_api/file_flows.cc b/src/file_api/file_flows.cc index e76a591a5..30fc60259 100644 --- a/src/file_api/file_flows.cc +++ b/src/file_api/file_flows.cc @@ -496,7 +496,8 @@ bool FileFlows::set_file_name(const uint8_t* fname, uint32_t name_size, uint64_t uint64_t multi_file_processing_id, const uint8_t* url, uint32_t url_size) { FileContext* context; - if (file_id) { + if (file_id) + { bool is_new_context = false; context = get_file_context(file_id, false, is_new_context, multi_file_processing_id); } diff --git a/src/file_api/file_lib.cc b/src/file_api/file_lib.cc index f72f6adb2..a3c9bf952 100644 --- a/src/file_api/file_lib.cc +++ b/src/file_api/file_lib.cc @@ -133,6 +133,8 @@ void FileInfo::copy(const FileInfo& other) pending_expire_time = other.pending_expire_time; // only one copy of file capture file_capture = nullptr; + policy_id = 0; + user_file_data = nullptr; } FileInfo::FileInfo(const FileInfo& other) @@ -461,6 +463,7 @@ void FileContext::check_policy(Flow* flow, FileDirection dir, FilePolicyBase* po bool FileContext::process(Packet* p, const uint8_t* file_data, int data_size, FilePosition position, FilePolicyBase* policy) { + // cppcheck-suppress unreadVariable Profile profile(file_perf_stats); Flow* flow = p->flow; @@ -624,12 +627,10 @@ bool FileContext::process(Packet* p, const uint8_t* file_data, int data_size, * 3) file magics are exhausted in depth * */ -void FileContext::find_file_type_from_ips(Packet* pkt, const uint8_t* file_data, int - data_size, +void FileContext::find_file_type_from_ips(Packet* pkt, const uint8_t* file_data, int data_size, FilePosition position) { bool depth_exhausted = false; - bool set_file_context = false; if ((int64_t)processed_bytes + data_size >= config->file_type_depth) { @@ -649,17 +650,20 @@ void FileContext::find_file_type_from_ips(Packet* pkt, const uint8_t* file_data, p->packet_flags |= PKT_ALLOW_MULTIPLE_DETECT; p->proto_bits |= PROTO_BIT__PDU; + bool set_file_context = false; FileFlows* files = FileFlows::get_file_flows(p->flow, false); - if (files and (!files->get_current_file_context() or files->get_current_file_context() != this)) + if (files) { - files->set_current_file_context(this); - set_file_context =true; + FileContext* context = files->get_current_file_context(); + if (!context or context != this) + { + files->set_current_file_context(this); + set_file_context = true; + } } fp_eval_service_group(p, conf->snort_protocol_id); if (set_file_context) - { files->set_current_file_context(nullptr); - } /* Check whether file transfer is done or type depth is reached */ if ((position == SNORT_FILE_END) || (position == SNORT_FILE_FULL) || depth_exhausted) finalize_file_type(); diff --git a/src/file_api/file_log.cc b/src/file_api/file_log.cc index 21c731041..94e6d3151 100644 --- a/src/file_api/file_log.cc +++ b/src/file_api/file_log.cc @@ -81,8 +81,8 @@ static void dl_tterm() class LogHandler : public DataHandler { public: - LogHandler(const FileLogConfig& conf) : DataHandler(s_name) - { config = conf; } + LogHandler(const FileLogConfig& conf) : DataHandler(s_name), config(conf) + { } void handle(DataEvent&, Flow*) override; @@ -202,7 +202,8 @@ void LogHandler::handle(DataEvent&, Flow* f) class FileLog : public Inspector { public: - FileLog(const FileLogConfig& conf) { config = conf; } + FileLog(const FileLogConfig& conf) : config(conf) + { } void show(const SnortConfig*) const override; void eval(Packet*) override { } diff --git a/src/filters/detection_filter.cc b/src/filters/detection_filter.cc index 39758fc8f..4de8f376f 100644 --- a/src/filters/detection_filter.cc +++ b/src/filters/detection_filter.cc @@ -57,6 +57,7 @@ void DetectionFilterConfigFree(DetectionFilterConfig* config) int detection_filter_test(void* pv, const SfIp* sip, const SfIp* dip, long curtime) { + // cppcheck-suppress unreadVariable RuleProfile profile(detectionFilterPerfStats); if (pv == nullptr) diff --git a/src/filters/sfrf.cc b/src/filters/sfrf.cc index 24005c9b8..57a389b52 100644 --- a/src/filters/sfrf.cc +++ b/src/filters/sfrf.cc @@ -375,6 +375,7 @@ static int SFRF_TestObject( switch (op) { case SFRF_COUNT_INCREMENT: + // cppcheck-suppress knownConditionTrueFalse if ( (dynNode->count+1) != 0 ) { dynNode->count++; diff --git a/src/flow/deferred_trust.h b/src/flow/deferred_trust.h index cfc4759d1..d8d641384 100644 --- a/src/flow/deferred_trust.h +++ b/src/flow/deferred_trust.h @@ -63,7 +63,7 @@ public: DeferredTrust() = default; ~DeferredTrust() = default; SO_PUBLIC void set_deferred_trust(unsigned module_id, bool on); - bool is_active() + bool is_active() const { return TRUST_DEFER_ON == deferred_trust || TRUST_DEFER_DEFERRING == deferred_trust; } bool try_trust() { @@ -71,7 +71,7 @@ public: deferred_trust = TRUST_DEFER_DEFERRING; return TRUST_DEFER_DEFERRING != deferred_trust; } - bool is_deferred() + bool is_deferred() const { return TRUST_DEFER_DEFERRING == deferred_trust; } void clear() { diff --git a/src/flow/expect_cache.cc b/src/flow/expect_cache.cc index 3dedd6606..769314ca2 100644 --- a/src/flow/expect_cache.cc +++ b/src/flow/expect_cache.cc @@ -159,8 +159,8 @@ ExpectNode* ExpectCache::find_node_by_packet(Packet* p, FlowKey &key) PktType type = p->type(); IpProtocol ip_proto = p->get_ip_proto_next(); - bool reversed_key = key.init(p->context->conf, type, ip_proto, dstIP, p->ptrs.dp, - srcIP, p->ptrs.sp, vlanId, mplsId, *p->pkth); + bool reversed_key = key.init(p->context->conf, type, ip_proto, srcIP, p->ptrs.sp, dstIP, p->ptrs.dp, + vlanId, mplsId, *p->pkth); /* Lookup order: @@ -182,17 +182,17 @@ ExpectNode* ExpectCache::find_node_by_packet(Packet* p, FlowKey &key) uint16_t port2; if (reversed_key) - { - port1 = key.port_l; - port2 = 0; - key.port_l = 0; - } - else { port1 = 0; port2 = key.port_h; key.port_h = 0; } + else + { + port1 = key.port_l; + port2 = 0; + key.port_l = 0; + } node = static_cast ( hash_table->get_user_data(&key) ); if (!node) { @@ -335,8 +335,14 @@ int ExpectCache::add_flow(const Packet *ctrlPkt, PktType type, IpProtocol ip_pro uint32_t mplsId = (ctrlPkt->proto_bits & PROTO_BIT__MPLS) ? ctrlPkt->ptrs.mplsHdr.label : 0; FlowKey key; - bool reversed_key = key.init(ctrlPkt->context->conf, type, ip_proto, cliIP, cliPort, - srvIP, srvPort, vlanId, mplsId, *ctrlPkt->pkth); + // This code assumes that the expected session is in the opposite direction of the control session + // when groups are significant + bool reversed_key = (ctrlPkt->pkth->flags & DAQ_PKT_FLAG_SIGNIFICANT_GROUPS) + ? key.init(ctrlPkt->context->conf, type, ip_proto, cliIP, cliPort, + srvIP, srvPort, vlanId, mplsId, ctrlPkt->pkth->address_space_id, ctrlPkt->pkth->egress_group, + ctrlPkt->pkth->ingress_group) + : key.init(ctrlPkt->context->conf, type, ip_proto, cliIP, cliPort, + srvIP, srvPort, vlanId, mplsId, *ctrlPkt->pkth); bool new_node = false; ExpectNode* node = static_cast ( hash_table->get_user_data(&key) ); diff --git a/src/flow/flow.cc b/src/flow/flow.cc index 35e1d54a1..58aa8b902 100644 --- a/src/flow/flow.cc +++ b/src/flow/flow.cc @@ -423,7 +423,7 @@ void Flow::set_expire(const Packet* p, uint64_t timeout) expire_time = (uint64_t)p->pkth->ts.tv_sec + timeout; } -bool Flow::expired(const Packet* p) +bool Flow::expired(const Packet* p) const { if ( !expire_time ) return false; @@ -505,7 +505,7 @@ Layer Flow::get_mpls_layer_per_dir(bool client) return mpls_server; } -bool Flow::is_pdu_inorder(uint8_t dir) +bool Flow::is_pdu_inorder(uint8_t dir) const { return ( (session != nullptr) && session->is_sequenced(dir) && (session->missing_in_reassembled(dir) == SSN_MISSING_NONE) diff --git a/src/flow/flow.h b/src/flow/flow.h index 3db187b97..7c9c5f480 100644 --- a/src/flow/flow.h +++ b/src/flow/flow.h @@ -208,14 +208,14 @@ public: void set_client_initiate(Packet*); void set_direction(Packet*); void set_expire(const Packet*, uint64_t timeout); - bool expired(const Packet*); + bool expired(const Packet*) const; void set_ttl(Packet*, bool client); void set_mpls_layer_per_dir(Packet*); Layer get_mpls_layer_per_dir(bool); void swap_roles(); void set_service(Packet*, const char* new_service); - bool get_attr(const std::string& key, int32_t& val); - bool get_attr(const std::string& key, std::string& val); + bool get_attr(const std::string& key, int32_t& val) const; + bool get_attr(const std::string& key, std::string& val) const; void set_attr(const std::string& key, const int32_t& val); void set_attr(const std::string& key, const std::string& val); // Use this API when the publisher of the attribute allocated memory for it and can give up its @@ -227,7 +227,7 @@ public: } template - bool get_attr(const std::string& key, T& val) + bool get_attr(const std::string& key, T& val) const { assert(stash); return stash->get(key, val); @@ -258,7 +258,7 @@ public: void set_to_client_detection(bool enable); void set_to_server_detection(bool enable); - int get_ignore_direction() + int get_ignore_direction() const { return ssn_state.ignore_direction; } int set_ignore_direction(char ignore_direction) @@ -267,20 +267,20 @@ public: return ssn_state.ignore_direction; } - bool two_way_traffic() + bool two_way_traffic() const { return (ssn_state.session_flags & SSNFLAG_SEEN_BOTH) == SSNFLAG_SEEN_BOTH; } - bool is_pdu_inorder(uint8_t dir); + bool is_pdu_inorder(uint8_t dir) const; bool is_direction_aborted(bool from_client) const; void set_proxied() { ssn_state.session_flags |= SSNFLAG_PROXIED; } - bool is_proxied() + bool is_proxied() const { return (ssn_state.session_flags & SSNFLAG_PROXIED) != 0; } - bool is_stream() + bool is_stream() const { return pkt_type == PktType::TCP or pkt_type == PktType::USER; } void block() @@ -386,13 +386,13 @@ public: void set_hard_expiration() { ssn_state.session_flags |= SSNFLAG_HARD_EXPIRATION; } - bool is_hard_expiration() + bool is_hard_expiration() const { return (ssn_state.session_flags & SSNFLAG_HARD_EXPIRATION) != 0; } void set_deferred_trust(unsigned module_id, bool on) { deferred_trust.set_deferred_trust(module_id, on); } - bool cannot_trust() + bool cannot_trust() const { return deferred_trust.is_active(); } bool try_trust() @@ -408,7 +408,7 @@ public: void trust(); - bool trust_is_deferred() + bool trust_is_deferred() const { return deferred_trust.is_deferred(); } void set_idle_timeout(unsigned timeout) diff --git a/src/flow/flow_stash.cc b/src/flow/flow_stash.cc index 318188d57..ef0e87a31 100644 --- a/src/flow/flow_stash.cc +++ b/src/flow/flow_stash.cc @@ -163,9 +163,9 @@ bool FlowStash::store(const SfIp& ip, const SnortConfig* sc) if ( sc->max_aux_ip > 0 ) { - for ( const auto& aip : aux_ip_fifo ) - if ( aip == ip ) - return false; + if ( std::any_of(aux_ip_fifo.cbegin(), aux_ip_fifo.cend(), + [ip](const snort::SfIp& aip){ return aip == ip; }) ) + return false; if ( aux_ip_fifo.size() == (unsigned)sc->max_aux_ip ) aux_ip_fifo.pop_back(); diff --git a/src/flow/ha.cc b/src/flow/ha.cc index 0af23ce06..0992d9482 100644 --- a/src/flow/ha.cc +++ b/src/flow/ha.cc @@ -121,10 +121,6 @@ static inline bool is_ip6_key(const FlowKey* key) FlowHAState::FlowHAState() { - state = INITIAL_STATE; - state |= (NEW | NEW_SESSION); - pending = NONE_PENDING; - // Set the initial update time to now+min_session_lifetime packet_gettimeofday(&next_update); timeradd(&next_update, &min_session_lifetime, &next_update); @@ -200,13 +196,11 @@ void FlowHAState::reset() init_next_update(); } -FlowHAClient::FlowHAClient(uint8_t length, bool session_client) +FlowHAClient::FlowHAClient(uint8_t length, bool session_client) : max_length(length) { if (!ha) return; - max_length = length; - if (session_client) { index = SESSION_HA_CLIENT_INDEX; diff --git a/src/flow/ha.h b/src/flow/ha.h index 92d02173e..38d36b0eb 100644 --- a/src/flow/ha.h +++ b/src/flow/ha.h @@ -83,8 +83,8 @@ private: static struct timeval min_sync_interval; struct timeval next_update; - uint16_t pending; - uint8_t state; + uint16_t pending = NONE_PENDING; + uint8_t state = NEW | NEW_SESSION; }; // Describe the message being produced or consumed. @@ -130,8 +130,8 @@ public: virtual bool is_update_required(snort::Flow*) { return false; } virtual uint8_t get_message_size(Flow&) { return max_length; } - FlowHAClientHandle handle; // Actual handle for the instance - uint8_t index; + FlowHAClientHandle handle = 0; // Actual handle for the instance + uint8_t index = 0; uint8_t max_length; protected: diff --git a/src/flow/session.h b/src/flow/session.h index 6af10e522..38d204d41 100644 --- a/src/flow/session.h +++ b/src/flow/session.h @@ -70,13 +70,13 @@ public: virtual void set_extra_data(snort::Packet*, uint32_t /*flag*/) { } - virtual bool is_sequenced(uint8_t /*dir*/) { return true; } - virtual bool are_packets_missing(uint8_t /*dir*/) { return false; } - virtual bool are_client_segments_queued() { return false; } + virtual bool is_sequenced(uint8_t /*dir*/) const { return true; } + virtual bool are_packets_missing(uint8_t /*dir*/) const { return false; } + virtual bool are_client_segments_queued() const { return false; } virtual void disable_reassembly(snort::Flow*) { } - virtual uint8_t get_reassembly_direction() { return SSN_DIR_NONE; } - virtual uint8_t missing_in_reassembled(uint8_t /*dir*/) { return SSN_MISSING_NONE; } + virtual uint8_t get_reassembly_direction() const { return SSN_DIR_NONE; } + virtual uint8_t missing_in_reassembled(uint8_t /*dir*/) const { return SSN_MISSING_NONE; } virtual bool set_packet_action_to_hold(snort::Packet*) { return false; } diff --git a/src/flow/test/CMakeLists.txt b/src/flow/test/CMakeLists.txt index 79dbbc9e7..b62e453f0 100644 --- a/src/flow/test/CMakeLists.txt +++ b/src/flow/test/CMakeLists.txt @@ -1,4 +1,6 @@ -add_cpputest( ha_test ) +add_cpputest( ha_test + SOURCES flow_stubs.h +) add_cpputest( deferred_trust_test SOURCES ../deferred_trust.cc @@ -11,6 +13,7 @@ add_cpputest( flow_stash_test add_cpputest( flow_control_test SOURCES ../flow_control.cc + flow_stubs.h ) add_cpputest( flow_cache_test @@ -18,6 +21,7 @@ add_cpputest( flow_cache_test ../flow_cache.cc ../flow_control.cc ../flow_key.cc + flow_stubs.h ../../hash/hash_key_operations.cc ../../hash/hash_lru_cache.cc ../../hash/primetable.cc @@ -31,4 +35,5 @@ add_cpputest( flow_test SOURCES ../flow.cc ../flow_data.cc + flow_stubs.h ) diff --git a/src/flow/test/flow_cache_test.cc b/src/flow/test/flow_cache_test.cc index 6ab9d5bb9..164bfb4c9 100644 --- a/src/flow/test/flow_cache_test.cc +++ b/src/flow/test/flow_cache_test.cc @@ -31,65 +31,37 @@ #include "flow/flow_cache.h" #include "flow/ha.h" #include "flow/session.h" -#include "main/policy.h" -#include "main/snort_config.h" -#include "main/thread_config.h" +#include "main/analyzer.h" #include "managers/inspector_manager.h" #include "packet_io/active.h" -#include "packet_tracer/packet_tracer.h" #include "protocols/icmp4.h" -#include "protocols/packet.h" #include "protocols/tcp.h" #include "protocols/udp.h" #include "protocols/vlan.h" -#include "stream/stream.h" #include "utils/util.h" #include "trace/trace_api.h" #include #include +#include "flow_stubs.h" + using namespace snort; THREAD_LOCAL bool Active::s_suspend = false; THREAD_LOCAL Active::ActiveSuspendReason Active::s_suspend_reason = Active::ASP_NONE; -THREAD_LOCAL PacketTracer* snort::s_pkt_trace = nullptr; THREAD_LOCAL const Trace* stream_trace = nullptr; void Active::drop_packet(snort::Packet const*, bool) { } -PacketTracer::~PacketTracer() = default; -void PacketTracer::log(const char*, ...) { } -void PacketTracer::open_file() { } -void PacketTracer::dump_to_daq(Packet*) { } -void PacketTracer::reset(bool) { } -void PacketTracer::pause() { } -void PacketTracer::unpause() { } +Analyzer* Analyzer::get_local_analyzer() { return nullptr; } +void Analyzer::resume(unsigned long) { } void Active::set_drop_reason(char const*) { } -Packet::Packet(bool) { } -Packet::~Packet() = default; -uint32_t Packet::get_flow_geneve_vni() const { return 0; } -Flow::~Flow() = default; -DetectionEngine::DetectionEngine() = default; +DetectionEngine::DetectionEngine() { context = nullptr; } ExpectCache::~ExpectCache() = default; DetectionEngine::~DetectionEngine() = default; -void Flow::init(PktType) { } -void Flow::flush(bool) { } -void Flow::reset(bool) { } -void Flow::free_flow_data() { } -void DataBus::publish(unsigned, unsigned, DataEvent&, Flow*) { } -void DataBus::publish(unsigned, unsigned, const uint8_t*, unsigned, Flow*) { } -void DataBus::publish(unsigned, unsigned, Packet*, Flow*) { } const SnortConfig* SnortConfig::get_conf() { return nullptr; } -void Flow::set_client_initiate(Packet*) { } -void Flow::set_direction(Packet*) { } -void set_network_policy(unsigned) { } -void set_inspection_policy(unsigned) { } -void set_ips_policy(const snort::SnortConfig*, unsigned) { } -void Flow::set_mpls_layer_per_dir(Packet*) { } void DetectionEngine::disable_all(Packet*) { } -void Stream::drop_traffic(const Packet*, char) { } -bool Stream::blocked_flow(Packet*) { return true; } ExpectCache::ExpectCache(uint32_t) { } bool ExpectCache::check(Packet*, Flow*) { return true; } bool ExpectCache::is_expected(Packet*) { return true; } @@ -103,29 +75,24 @@ void ThreadConfig::preemptive_kick() {} namespace snort { -NetworkPolicy* get_network_policy() { return nullptr; } -InspectionPolicy* get_inspection_policy() { return nullptr; } -IpsPolicy* get_ips_policy() { return nullptr; } -unsigned SnortConfig::get_thread_reload_id() { return 0; } +Flow::~Flow() = default; +void Flow::init(PktType) { } +void Flow::flush(bool) { } +void Flow::reset(bool) { } +void Flow::free_flow_data() { } +void Flow::set_client_initiate(Packet*) { } +void Flow::set_direction(Packet*) { } +void Flow::set_mpls_layer_per_dir(Packet*) { } -namespace layer -{ -const vlan::VlanTagHdr* get_vlan_layer(const Packet* const) { return nullptr; } -} time_t packet_time() { return 0; } -} +void packet_gettimeofday(struct timeval* tv) { *tv = {}; } -namespace snort -{ namespace ip { uint32_t IpApi::id() const { return 0; } } } -void Stream::stop_inspection(Flow*, Packet*, char, int32_t, int) { } - - int ExpectCache::add_flow(const Packet*, PktType, IpProtocol, const SfIp*, uint16_t, const SfIp*, uint16_t, char, FlowData*, SnortProtocolId, bool, bool, bool, bool) { diff --git a/src/flow/test/flow_control_test.cc b/src/flow/test/flow_control_test.cc index cf83b6b8f..d4803eb08 100644 --- a/src/flow/test/flow_control_test.cc +++ b/src/flow/test/flow_control_test.cc @@ -47,29 +47,19 @@ #include #include +#include "flow_stubs.h" + using namespace snort; THREAD_LOCAL bool Active::s_suspend = false; THREAD_LOCAL Active::ActiveSuspendReason Active::s_suspend_reason = Active::ASP_NONE; -THREAD_LOCAL PacketTracer* snort::s_pkt_trace = nullptr; - void Active::drop_packet(snort::Packet const*, bool) { } -PacketTracer::~PacketTracer() = default; -void PacketTracer::log(const char*, ...) { } -void PacketTracer::open_file() { } -void PacketTracer::dump_to_daq(Packet*) { } -void PacketTracer::reset(bool) { } -void PacketTracer::pause() { } -void PacketTracer::unpause() { } void Active::set_drop_reason(char const*) { } -Packet::Packet(bool) { } -Packet::~Packet() = default; -uint32_t Packet::get_flow_geneve_vni() const { return 0; } FlowCache::FlowCache(const FlowCacheConfig& cfg) : config(cfg) { } FlowCache::~FlowCache() = default; Flow::~Flow() = default; -DetectionEngine::DetectionEngine() = default; +DetectionEngine::DetectionEngine() { context = nullptr; } DetectionEngine::~DetectionEngine() = default; ExpectCache::~ExpectCache() = default; unsigned FlowCache::purge() { return 1; } @@ -85,38 +75,17 @@ size_t FlowCache::uni_flows_size() const { return 0; } size_t FlowCache::uni_ip_flows_size() const { return 0; } size_t FlowCache::flows_size() const { return 0; } void Flow::init(PktType) { } -void DataBus::publish(unsigned, unsigned, DataEvent&, Flow*) { } -void DataBus::publish(unsigned, unsigned, const uint8_t*, unsigned, Flow*) { } -void DataBus::publish(unsigned, unsigned, Packet*, Flow*) { } const SnortConfig* SnortConfig::get_conf() { return nullptr; } void FlowCache::unlink_uni(Flow*) { } void Flow::set_client_initiate(Packet*) { } void Flow::set_direction(Packet*) { } -void set_network_policy(unsigned) { } -void set_inspection_policy(unsigned) { } -void set_ips_policy(const snort::SnortConfig*, unsigned) { } void Flow::set_mpls_layer_per_dir(Packet*) { } void DetectionEngine::disable_all(Packet*) { } -void Stream::drop_traffic(const Packet*, char) { } -bool Stream::blocked_flow(Packet*) { return true; } ExpectCache::ExpectCache(uint32_t) { } bool ExpectCache::check(Packet*, Flow*) { return true; } bool ExpectCache::is_expected(Packet*) { return true; } Flow* HighAvailabilityManager::import(Packet&, FlowKey&) { return nullptr; } -namespace snort -{ -NetworkPolicy* get_network_policy() { return nullptr; } -InspectionPolicy* get_inspection_policy() { return nullptr; } -IpsPolicy* get_ips_policy() { return nullptr; } -unsigned SnortConfig::get_thread_reload_id() { return 0; } - -namespace layer -{ -const vlan::VlanTagHdr* get_vlan_layer(const Packet* const) { return nullptr; } -} -} - namespace snort { namespace ip @@ -167,8 +136,6 @@ bool FlowKey::init( return true; } -void Stream::stop_inspection(Flow*, Packet*, char, int32_t, int) { } - int ExpectCache::add_flow(const Packet*, PktType, IpProtocol, const SfIp*, uint16_t, diff --git a/src/flow/test/flow_stash_test.cc b/src/flow/test/flow_stash_test.cc index a7d5fde73..c410f32ab 100644 --- a/src/flow/test/flow_stash_test.cc +++ b/src/flow/test/flow_stash_test.cc @@ -79,7 +79,11 @@ static SnortConfig snort_conf; namespace snort { -SnortConfig::SnortConfig(const SnortConfig* const, const char*) { } +SnortConfig::SnortConfig(const SnortConfig* const, const char*) +{ + daq_config = nullptr; + thread_config = nullptr; +} SnortConfig::~SnortConfig() = default; const SnortConfig* SnortConfig::get_conf() { return &snort_conf; } diff --git a/src/flow/test/flow_stubs.h b/src/flow/test/flow_stubs.h new file mode 100644 index 000000000..9a752fc98 --- /dev/null +++ b/src/flow/test/flow_stubs.h @@ -0,0 +1,84 @@ +//-------------------------------------------------------------------------- +// Copyright (C) 2020-2023 Cisco and/or its affiliates. All rights reserved. +// +// This program is free software; you can redistribute it and/or modify it +// under the terms of the GNU General Public License Version 2 as published +// by the Free Software Foundation. You may not use, modify or distribute +// this program under any other version of the GNU General Public License. +// +// This program is distributed in the hope that it will be useful, but +// WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// General Public License for more details. +// +// You should have received a copy of the GNU General Public License along +// with this program; if not, write to the Free Software Foundation, Inc., +// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +//-------------------------------------------------------------------------- +// flow_stubs.h author Ron Dempster + +#ifndef FLOW_STUBS_H +#define FLOW_STUBS_H + +#include "framework/data_bus.h" +#include "log/messages.h" +#include "main/policy.h" +#include "main/snort_config.h" +#include "main/thread_config.h" +#include "packet_tracer/packet_tracer.h" +#include "protocols/layer.h" +#include "protocols/packet.h" +#include "stream/stream.h" + +namespace snort +{ +void ErrorMessage(const char*,...) { } +void LogMessage(const char*,...) { } + +void DataBus::publish(unsigned, unsigned, DataEvent&, Flow*) { } +void DataBus::publish(unsigned, unsigned, const uint8_t*, unsigned, Flow*) { } +void DataBus::publish(unsigned, unsigned, Packet*, Flow*) { } + +Packet::Packet(bool) +{ + memset((char*) this , 0, sizeof(*this)); + ip_proto_next = IpProtocol::PROTO_NOT_SET; + packet_flags = PKT_FROM_CLIENT; +} +Packet::~Packet() = default; +uint32_t Packet::get_flow_geneve_vni() const { return 0; } + +THREAD_LOCAL PacketTracer* s_pkt_trace = nullptr; + +PacketTracer::~PacketTracer() = default; +void PacketTracer::log(const char*, ...) { } +void PacketTracer::open_file() { } +void PacketTracer::dump_to_daq(Packet*) { } +void PacketTracer::reset(bool) { } +void PacketTracer::pause() { } +void PacketTracer::unpause() { } + +namespace layer +{ +const vlan::VlanTagHdr* get_vlan_layer(const Packet* const) { return nullptr; } +} + +void Stream::drop_traffic(const Packet*, char) { } +bool Stream::blocked_flow(Packet*) { return true; } +void Stream::stop_inspection(Flow*, Packet*, char, int32_t, int) { } + +NetworkPolicy* get_network_policy() { return nullptr; } +InspectionPolicy* get_inspection_policy() { return nullptr; } +IpsPolicy* get_ips_policy() { return nullptr; } +void set_network_policy(NetworkPolicy*) { } +void set_inspection_policy(InspectionPolicy*) { } +void set_ips_policy(IpsPolicy*) { } +unsigned SnortConfig::get_thread_reload_id() { return 0; } +} + +void set_network_policy(unsigned) { } +void set_inspection_policy(unsigned) { } +void set_ips_policy(const snort::SnortConfig*, unsigned) { } +void select_default_policy(const _daq_pkt_hdr&, const snort::SnortConfig*) { } + +#endif diff --git a/src/flow/test/flow_test.cc b/src/flow/test/flow_test.cc index 72502b065..3a7889e31 100644 --- a/src/flow/test/flow_test.cc +++ b/src/flow/test/flow_test.cc @@ -40,10 +40,9 @@ #include #include -using namespace snort; +#include "flow_stubs.h" -Packet::Packet(bool) { } -Packet::~Packet() = default; +using namespace snort; void Inspector::rem_ref() {} @@ -61,28 +60,13 @@ void FlowStash::reset() {} void DetectionEngine::onload(Flow*) {} -void set_network_policy(unsigned) { } -void set_inspection_policy(unsigned) { } -void set_ips_policy(const snort::SnortConfig*, unsigned) { } -void select_default_policy(const _daq_pkt_hdr&, const SnortConfig*) { } -namespace snort -{ -NetworkPolicy* get_network_policy() { return nullptr; } -InspectionPolicy* get_inspection_policy() { return nullptr; } -IpsPolicy* get_ips_policy() { return nullptr; } -void set_network_policy(NetworkPolicy*) { } -void set_inspection_policy(InspectionPolicy*) { } -void set_ips_policy(IpsPolicy*) { } -unsigned SnortConfig::get_thread_reload_id() { return 0; } -} - Packet* DetectionEngine::set_next_packet(const Packet*, Flow*) { return nullptr; } ContextSwitcher* Analyzer::get_switcher() { return nullptr; } snort::IpsContext* ContextSwitcher::get_context() const { return nullptr; } IpsContext* DetectionEngine::get_context() { return nullptr; } -DetectionEngine::DetectionEngine() = default; +DetectionEngine::DetectionEngine() { context = nullptr; } DetectionEngine::~DetectionEngine() = default; @@ -93,19 +77,10 @@ uint8_t ip::IpApi::ttl() const { return 0; } const Layer* layer::get_mpls_layer(const Packet* const) { return nullptr; } -void DataBus::publish(unsigned, unsigned, Packet*, Flow*) {} - const SnortConfig* SnortConfig::get_conf() { return nullptr; } TEST_GROUP(nondefault_timeout) { - void setup() override - { - } - - void teardown() override - { - } }; TEST(nondefault_timeout, hard_expiration) diff --git a/src/flow/test/ha_test.cc b/src/flow/test/ha_test.cc index e94af042d..55b16b091 100644 --- a/src/flow/test/ha_test.cc +++ b/src/flow/test/ha_test.cc @@ -31,6 +31,8 @@ using namespace snort; +#include "flow_stubs.h" + #define MSG_SIZE 100 #define TEST_KEY 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47 @@ -180,9 +182,6 @@ Flow* Stream::get_flow(const FlowKey* flowkey) return (Flow*)mock().getData("flow").getObjectPointer(); } -Packet::Packet(bool) { } -Packet::~Packet() = default; - void Stream::delete_flow(const FlowKey* flowkey) { mock().actualCall("delete_flow"); @@ -193,8 +192,9 @@ void Stream::delete_flow(const FlowKey* flowkey) namespace snort { -void ErrorMessage(const char*,...) { } -void LogMessage(const char*,...) { } +Flow::~Flow() = default; +void Flow::set_client_initiate(Packet*) { } +void Flow::set_direction(Packet*) { } void packet_gettimeofday(struct timeval* tv) { @@ -206,20 +206,13 @@ bool FlowKey::is_equal(const void*, const void*, size_t) { return false; } int SFDAQInstance::ioctl(DAQ_IoctlCmd, void*, size_t) { return DAQ_SUCCESS; } -Flow::~Flow() { } - FlowStash::~FlowStash() = default; -void Flow::set_client_initiate(Packet*) { } -void Flow::set_direction(Packet*) { } - SideChannel* SideChannelManager::get_side_channel(SCPort) { return (SideChannel*)mock().getData("s_side_channel").getObjectPointer(); } -SideChannel::SideChannel() = default; - Connector::Direction SideChannel::get_direction() { return Connector::CONN_DUPLEX; } @@ -395,8 +388,8 @@ TEST_GROUP(high_availability_test) { Flow s_flow; Active active; - StreamHAClient* s_ha_client; // cppcheck-suppress variableScope - FlowHAClient* s_other_ha_client; // cppcheck-suppress variableScope + StreamHAClient* s_ha_client; + FlowHAClient* s_other_ha_client; uint8_t s_message[MSG_SIZE]; SCMessage s_sc_message; Packet s_pkt; @@ -427,7 +420,7 @@ TEST_GROUP(high_availability_test) s_sc_message = {}; s_sc_message.content = s_message; mock().setDataObject("message_content", "SCMessage", &s_sc_message); - s_pkt.active = &active; // cppcheck-suppress unreadVariable + s_pkt.active = &active; memset(&ha_stats, 0, sizeof(ha_stats)); @@ -440,8 +433,8 @@ TEST_GROUP(high_availability_test) HighAvailabilityManager::configure(&hac); HighAvailabilityManager::thread_init(); - s_ha_client = new StreamHAClient; // cppcheck-suppress unreadVariable - s_other_ha_client = new OtherHAClient; // cppcheck-suppress unreadVariable + s_ha_client = new StreamHAClient; + s_other_ha_client = new OtherHAClient; } void teardown() override diff --git a/src/framework/connector.h b/src/framework/connector.h index e0af235f1..28e626139 100644 --- a/src/framework/connector.h +++ b/src/framework/connector.h @@ -74,7 +74,7 @@ public: virtual Direction get_connector_direction() = 0; const std::string connector_name; - const ConnectorConfig* config; + const ConnectorConfig* config = nullptr; protected: Connector() = default; diff --git a/src/framework/cursor.cc b/src/framework/cursor.cc index 4ab3a7864..25cfc96fa 100644 --- a/src/framework/cursor.cc +++ b/src/framework/cursor.cc @@ -53,6 +53,7 @@ Cursor::Cursor(const Cursor& rhs) if (rhs.data) { data = new CursorDataVec; + data->reserve(rhs.data->size()); for (CursorData*& cd : *rhs.data) data->push_back(cd->clone()); @@ -79,10 +80,10 @@ void Cursor::set_data(CursorData* cd) if (data) { - unsigned id = cd->get_id(); + unsigned i = cd->get_id(); for (CursorData*& old : *data) { - if (old->get_id() == id) + if (old->get_id() == i) { delete old; old = cd; @@ -102,11 +103,11 @@ void Cursor::reset(Packet* p) { if (p->flow and p->flow->gadget) { - const DataBuffer& buf = DetectionEngine::get_alt_buffer(p); + const DataBuffer& alt_buf = DetectionEngine::get_alt_buffer(p); - if (buf.len) + if (alt_buf.len) { - set("alt_data", buf.data, buf.len); + set("alt_data", alt_buf.data, alt_buf.len); return; } } diff --git a/src/framework/cursor.h b/src/framework/cursor.h index 42cfaade0..ac96a64e5 100644 --- a/src/framework/cursor.h +++ b/src/framework/cursor.h @@ -44,7 +44,7 @@ public: virtual ~CursorData() = default; virtual CursorData* clone() = 0; - unsigned get_id() + unsigned get_id() const { return id; } static unsigned create_cursor_data_id() diff --git a/src/framework/data_bus.cc b/src/framework/data_bus.cc index f9d0e7ae0..d24cd5c19 100644 --- a/src/framework/data_bus.cc +++ b/src/framework/data_bus.cc @@ -78,7 +78,7 @@ DataBus::DataBus() = default; DataBus::~DataBus() { - for ( auto& p : pub_sub ) + for ( const auto& p : pub_sub ) { for ( auto* h : p ) { diff --git a/src/framework/decode_data.h b/src/framework/decode_data.h index ff0b26cbe..93c4da995 100644 --- a/src/framework/decode_data.h +++ b/src/framework/decode_data.h @@ -118,25 +118,30 @@ struct DecodeData * these three pointers are each referenced literally * dozens if not hundreds of times. NOTHING else should be added!! */ - const snort::tcp::TCPHdr* tcph; - const snort::udp::UDPHdr* udph; - const snort::icmp::ICMPHdr* icmph; + const snort::tcp::TCPHdr* tcph = nullptr; + const snort::udp::UDPHdr* udph = nullptr; + const snort::icmp::ICMPHdr* icmph = nullptr; - uint16_t sp; /* source port (TCP/UDP) */ - uint16_t dp; /* dest port (TCP/UDP) */ + uint16_t sp = 0; /* source port (TCP/UDP) */ + uint16_t dp = 0; /* dest port (TCP/UDP) */ - uint16_t decode_flags; - PktType type; + uint16_t decode_flags = 0; + PktType type = PktType::NONE; snort::ip::IpApi ip_api; - snort::mpls::MplsHdr mplsHdr; // FIXIT-L need to zero this? + snort::mpls::MplsHdr mplsHdr = {}; inline void reset() { - memset(this, 0, offsetof(DecodeData, ip_api)); - ip_api.reset(); + tcph = nullptr; + udph = nullptr; + icmph = nullptr; sp = 0; dp = 0; + decode_flags = 0; + type = PktType::NONE; + ip_api.reset(); + mplsHdr = {}; } inline void set_pkt_type(PktType pkt_type) diff --git a/src/framework/inspector.cc b/src/framework/inspector.cc index 754312990..92229a42e 100644 --- a/src/framework/inspector.cc +++ b/src/framework/inspector.cc @@ -58,12 +58,8 @@ Inspector::Inspector() Inspector::~Inspector() { - unsigned total = 0; - for (unsigned i = 0; i < ThreadConfig::get_instance_max(); ++i ) - total += ref_count[i]; - - assert(!total); + assert(0 == ref_count[i]); delete[] ref_count; } diff --git a/src/framework/ips_option.cc b/src/framework/ips_option.cc index c6fed3a24..1df0f462a 100644 --- a/src/framework/ips_option.cc +++ b/src/framework/ips_option.cc @@ -97,18 +97,18 @@ TEST_CASE("IpsOption test", "[ips_option]") SECTION("hash test") { - StubIpsOption main_ips("ips_test", + StubIpsOption other_main_ips("ips_test", option_type_t::RULE_OPTION_TYPE_OTHER); SECTION("hash test with short string") { StubIpsOption main_ips_short("ips_test", option_type_t::RULE_OPTION_TYPE_OTHER); - REQUIRE((main_ips.hash() == main_ips_short.hash()) == true); + REQUIRE((other_main_ips.hash() == main_ips_short.hash()) == true); StubIpsOption main_ips_short_diff("not_ips_test", option_type_t::RULE_OPTION_TYPE_OTHER); - REQUIRE((main_ips.hash() == main_ips_short_diff.hash()) == false); + REQUIRE((other_main_ips.hash() == main_ips_short_diff.hash()) == false); } SECTION("hash test with long string") @@ -134,7 +134,7 @@ TEST_CASE("IpsOption test", "[ips_option]") option_type_t::RULE_OPTION_TYPE_OTHER); REQUIRE(main_ips_long_first.hash() == main_ips_long_second.hash()); - REQUIRE(main_ips_long_first.hash() != main_ips.hash()); + REQUIRE(main_ips_long_first.hash() != other_main_ips.hash()); } } } diff --git a/src/framework/logger.h b/src/framework/logger.h index 16073b88d..f577d1690 100644 --- a/src/framework/logger.h +++ b/src/framework/logger.h @@ -70,7 +70,7 @@ protected: Logger() = default; private: - const LogApi* api; + const LogApi* api = nullptr; }; typedef Logger* (* LogNewFunc)(class Module*); diff --git a/src/framework/lua_api.h b/src/framework/lua_api.h index fbd5f2b76..741f24293 100644 --- a/src/framework/lua_api.h +++ b/src/framework/lua_api.h @@ -39,11 +39,8 @@ public: std::string chunk; protected: - LuaApi(const std::string& s, const std::string& c) - { - name = s; - chunk = c; - } + LuaApi(const std::string& s, const std::string& c) : name(s), chunk(c) + { } }; #endif diff --git a/src/framework/mpse.cc b/src/framework/mpse.cc index 665720c20..dfb333a25 100644 --- a/src/framework/mpse.cc +++ b/src/framework/mpse.cc @@ -43,12 +43,8 @@ namespace snort // base stuff //------------------------------------------------------------------------- -Mpse::Mpse(const char* m) -{ - method = m; - verbose = 0; - api = nullptr; -} +Mpse::Mpse(const char* m) : method(m) +{ } int Mpse::search( const unsigned char* T, int n, MpseMatch match, diff --git a/src/framework/mpse.h b/src/framework/mpse.h index 8f9d5f729..2f2915cfc 100644 --- a/src/framework/mpse.h +++ b/src/framework/mpse.h @@ -120,8 +120,8 @@ protected: private: std::string method; - int verbose; - const MpseApi* api; + int verbose = 0; + const MpseApi* api = nullptr; }; typedef void (* MpseOptFunc)(SnortConfig*); diff --git a/src/framework/packet_constraints.cc b/src/framework/packet_constraints.cc index 0d4a48c21..99f59fae2 100644 --- a/src/framework/packet_constraints.cc +++ b/src/framework/packet_constraints.cc @@ -171,7 +171,7 @@ TEST_CASE("Packet constraints matching", "[framework]") cs.src_ip = sip; cs.dst_ip = dip; - CHECK( match_constraints(cs, sip, dip, sport, dport) ); + CHECK( true == match_constraints(cs, sip, dip, sport, dport) ); } SECTION("backwards") @@ -188,7 +188,7 @@ TEST_CASE("Packet constraints matching", "[framework]") cs.src_ip = sip; cs.dst_ip = dip; - CHECK( !match_constraints(cs, dip, sip, dport, sport) ); + CHECK( false == match_constraints(cs, dip, sip, dport, sport) ); } SECTION("any ip") @@ -201,7 +201,7 @@ TEST_CASE("Packet constraints matching", "[framework]") cs.src_port = sport; cs.dst_port = dport; - CHECK( match_constraints(cs, sip, dip, sport, dport) ); + CHECK( true == match_constraints(cs, sip, dip, sport, dport) ); } SECTION("any port") @@ -214,7 +214,7 @@ TEST_CASE("Packet constraints matching", "[framework]") cs.src_ip = sip; cs.dst_ip = dip; - CHECK( match_constraints(cs, sip, dip, sport, dport) ); + CHECK( true == match_constraints(cs, sip, dip, sport, dport) ); } SECTION("any src") @@ -227,7 +227,7 @@ TEST_CASE("Packet constraints matching", "[framework]") cs.dst_port = dport; cs.dst_ip = dip; - CHECK( match_constraints(cs, sip, dip, sport, dport) ); + CHECK( true == match_constraints(cs, sip, dip, sport, dport) ); } } diff --git a/src/framework/range.cc b/src/framework/range.cc index 06b31d899..9d49ef65a 100644 --- a/src/framework/range.cc +++ b/src/framework/range.cc @@ -332,151 +332,151 @@ TEST_CASE("dflt op", "[RangeCheck]") { RangeCheck rc; - REQUIRE(rc.parse("5")); + REQUIRE(true == rc.parse("5")); REQUIRE(rc.op == RangeCheck::EQ); REQUIRE((rc.max == 5)); - CHECK(rc.eval(5)); + CHECK(true == rc.eval(5)); - CHECK(!rc.eval(4)); - CHECK(!rc.eval(6)); + CHECK(false == rc.eval(4)); + CHECK(false == rc.eval(6)); } TEST_CASE("=", "[RangeCheck]") { RangeCheck rc; - REQUIRE(rc.parse("=+0x5")); + REQUIRE(true == rc.parse("=+0x5")); REQUIRE(rc.op == RangeCheck::EQ); REQUIRE((rc.max == 5)); - CHECK(rc.eval(5)); + CHECK(true == rc.eval(5)); - CHECK(!rc.eval(4)); - CHECK(!rc.eval(6)); + CHECK(false == rc.eval(4)); + CHECK(false == rc.eval(6)); } TEST_CASE("!", "[RangeCheck]") { RangeCheck rc; - REQUIRE(rc.parse("!-5")); + REQUIRE(true == rc.parse("!-5")); REQUIRE(rc.op == RangeCheck::NOT); REQUIRE((rc.max == -5)); - CHECK(rc.eval(-4)); - CHECK(rc.eval(-6)); + CHECK(true == rc.eval(-4)); + CHECK(true == rc.eval(-6)); - CHECK(!rc.eval(-5)); + CHECK(false == rc.eval(-5)); } TEST_CASE("!=", "[RangeCheck]") { RangeCheck rc; - REQUIRE(rc.parse("!=5")); + REQUIRE(true == rc.parse("!=5")); REQUIRE(rc.op == RangeCheck::NOT); REQUIRE((rc.max == 5)); - CHECK(rc.eval(4)); - CHECK(rc.eval(6)); + CHECK(true == rc.eval(4)); + CHECK(true == rc.eval(6)); - CHECK(!rc.eval(5)); + CHECK(false == rc.eval(5)); } TEST_CASE("<", "[RangeCheck]") { RangeCheck rc; - REQUIRE(rc.parse("<5")); + REQUIRE(true == rc.parse("<5")); REQUIRE(rc.op == RangeCheck::LT); REQUIRE((rc.max == 5)); - CHECK(rc.eval(4)); - CHECK(rc.eval(-1)); + CHECK(true == rc.eval(4)); + CHECK(true == rc.eval(-1)); - CHECK(!rc.eval(5)); - CHECK(!rc.eval(6)); + CHECK(false == rc.eval(5)); + CHECK(false == rc.eval(6)); } TEST_CASE("<=", "[RangeCheck]") { RangeCheck rc; - REQUIRE(rc.parse("<=5")); + REQUIRE(true == rc.parse("<=5")); REQUIRE(rc.op == RangeCheck::LE); REQUIRE((rc.max == 5)); - CHECK(rc.eval(5)); - CHECK(rc.eval(-1)); + CHECK(true == rc.eval(5)); + CHECK(true == rc.eval(-1)); - CHECK(!rc.eval(6)); - CHECK(!rc.eval(1000)); + CHECK(false == rc.eval(6)); + CHECK(false == rc.eval(1000)); } TEST_CASE(">", "[RangeCheck]") { RangeCheck rc; - REQUIRE(rc.parse(">5")); + REQUIRE(true == rc.parse(">5")); REQUIRE((rc.op == RangeCheck::GT)); REQUIRE((rc.min == 5)); - CHECK(rc.eval(6)); - CHECK(rc.eval(10)); + CHECK(true == rc.eval(6)); + CHECK(true == rc.eval(10)); - CHECK(!rc.eval(5)); - CHECK(!rc.eval(-1)); + CHECK(false == rc.eval(5)); + CHECK(false == rc.eval(-1)); } TEST_CASE(">=", "[RangeCheck]") { RangeCheck rc; - REQUIRE(rc.parse(">=5")); + REQUIRE(true == rc.parse(">=5")); REQUIRE((rc.op == RangeCheck::GE)); REQUIRE((rc.min == 5)); - CHECK(rc.eval(5)); - CHECK(rc.eval(10)); + CHECK(true == rc.eval(5)); + CHECK(true == rc.eval(10)); - CHECK(!rc.eval(4)); - CHECK(!rc.eval(-4)); + CHECK(false == rc.eval(4)); + CHECK(false == rc.eval(-4)); } TEST_CASE("<>", "[RangeCheck]") { RangeCheck rc; - REQUIRE(rc.parse("0<>5")); + REQUIRE(true == rc.parse("0<>5")); REQUIRE((rc.op == RangeCheck::LG)); REQUIRE(rc.min == 0); REQUIRE((rc.max == 5)); - CHECK(rc.eval(1)); - CHECK(rc.eval(4)); + CHECK(true == rc.eval(1)); + CHECK(true == rc.eval(4)); - CHECK(!rc.eval(-1)); - CHECK(!rc.eval(0)); - CHECK(!rc.eval(5)); - CHECK(!rc.eval(6)); + CHECK(false == rc.eval(-1)); + CHECK(false == rc.eval(0)); + CHECK(false == rc.eval(5)); + CHECK(false == rc.eval(6)); } TEST_CASE("<=>", "[RangeCheck]") { RangeCheck rc; - REQUIRE(rc.parse("0<=>5")); + REQUIRE(true == rc.parse("0<=>5")); REQUIRE((rc.op == RangeCheck::LEG)); REQUIRE((rc.max == 5)); - CHECK(rc.eval(0)); - CHECK(rc.eval(1)); - CHECK(rc.eval(4)); - CHECK(rc.eval(5)); + CHECK(true == rc.eval(0)); + CHECK(true == rc.eval(1)); + CHECK(true == rc.eval(4)); + CHECK(true == rc.eval(5)); - CHECK(!rc.eval(-1)); - CHECK(!rc.eval(6)); + CHECK(false == rc.eval(-1)); + CHECK(false == rc.eval(6)); } TEST_CASE("parsing", "[RangeCheck]") @@ -487,63 +487,63 @@ TEST_CASE("parsing", "[RangeCheck]") { SECTION("a") { - REQUIRE(rc.parse("5")); + REQUIRE(true == rc.parse("5")); CHECK(rc.op == RangeCheck::EQ); CHECK((rc.max == 5)); } SECTION("b") { - REQUIRE(rc.parse(" 5 ")); + REQUIRE(true == rc.parse(" 5 ")); CHECK(rc.op == RangeCheck::EQ); CHECK((rc.max == 5)); } SECTION("c") { - REQUIRE(rc.parse(" ! 5 ")); + REQUIRE(true == rc.parse(" ! 5 ")); CHECK(rc.op == RangeCheck::NOT); CHECK((rc.max == 5)); } SECTION("d") { - REQUIRE(rc.parse(" != 5 ")); + REQUIRE(true == rc.parse(" != 5 ")); CHECK(rc.op == RangeCheck::NOT); CHECK((rc.max == 5)); } SECTION("e") { - REQUIRE(rc.parse(" < 5 ")); + REQUIRE(true == rc.parse(" < 5 ")); CHECK((rc.op == RangeCheck::LT)); CHECK((rc.max == 5)); } SECTION("f") { - REQUIRE(rc.parse(" > 5 ")); + REQUIRE(true == rc.parse(" > 5 ")); CHECK((rc.op == RangeCheck::GT)); CHECK((rc.min == 5)); } SECTION("g") { - REQUIRE(rc.parse(" <= 5 ")); + REQUIRE(true == rc.parse(" <= 5 ")); CHECK((rc.op == RangeCheck::LE)); CHECK((rc.max == 5)); } SECTION("h") { - REQUIRE(rc.parse(" >= 5 ")); + REQUIRE(true == rc.parse(" >= 5 ")); CHECK((rc.op == RangeCheck::GE)); CHECK((rc.min == 5)); } SECTION("i") { - REQUIRE(rc.parse(" 10 <> 50 ")); + REQUIRE(true == rc.parse(" 10 <> 50 ")); CHECK((rc.op == RangeCheck::LG)); CHECK((rc.min == 10)); CHECK((rc.max == 50)); @@ -551,7 +551,7 @@ TEST_CASE("parsing", "[RangeCheck]") SECTION("j") { - REQUIRE(rc.parse(" 0x10 <=> 0x50 ")); + REQUIRE(true == rc.parse(" 0x10 <=> 0x50 ")); CHECK((rc.op == RangeCheck::LEG)); CHECK((rc.min == 0x10)); CHECK((rc.max == 0x50)); @@ -559,7 +559,7 @@ TEST_CASE("parsing", "[RangeCheck]") SECTION("k") { - REQUIRE(rc.parse(" -0123 <=> 0x123 ")); + REQUIRE(true == rc.parse(" -0123 <=> 0x123 ")); CHECK((rc.op == RangeCheck::LEG)); CHECK((rc.min == -83)); CHECK((rc.max == 291)); @@ -569,41 +569,41 @@ TEST_CASE("parsing", "[RangeCheck]") SECTION("invalid ranges") { // spacey operators - CHECK(!rc.parse(" ! = 5 ")); - CHECK(!rc.parse(" < = 5 ")); - CHECK(!rc.parse(" > = 5 ")); - CHECK(!rc.parse(" 1 < > 5 ")); - CHECK(!rc.parse(" 1 < = > 5 ")); - CHECK(!rc.parse(" < > 5 ")); - CHECK(!rc.parse(" < = > 5 ")); + CHECK(false == rc.parse(" ! = 5 ")); + CHECK(false == rc.parse(" < = 5 ")); + CHECK(false == rc.parse(" > = 5 ")); + CHECK(false == rc.parse(" 1 < > 5 ")); + CHECK(false == rc.parse(" 1 < = > 5 ")); + CHECK(false == rc.parse(" < > 5 ")); + CHECK(false == rc.parse(" < = > 5 ")); // other invalids - CHECK(!rc.parse("5x")); - CHECK(!rc.parse("5.0")); - CHECK(!rc.parse("5-0")); - CHECK(!rc.parse("*5")); - CHECK(!rc.parse("=$5")); - CHECK(!rc.parse("=5x")); - CHECK(!rc.parse("<<0")); - CHECK(!rc.parse("+9223372036854775808")); - CHECK(!rc.parse("-9223372036854775809")); - CHECK(!rc.parse("4<>2")); - CHECK(!rc.parse("24<=>16")); + CHECK(false == rc.parse("5x")); + CHECK(false == rc.parse("5.0")); + CHECK(false == rc.parse("5-0")); + CHECK(false == rc.parse("*5")); + CHECK(false == rc.parse("=$5")); + CHECK(false == rc.parse("=5x")); + CHECK(false == rc.parse("<<0")); + CHECK(false == rc.parse("+9223372036854775808")); + CHECK(false == rc.parse("-9223372036854775809")); + CHECK(false == rc.parse("4<>2")); + CHECK(false == rc.parse("24<=>16")); // backwards - CHECK(!rc.parse(" 5 = ")); - CHECK(!rc.parse(" 5 ! ")); - CHECK(!rc.parse(" 5 != ")); - CHECK(!rc.parse(" 5 < ")); - CHECK(!rc.parse(" 5 <= ")); - CHECK(!rc.parse(" 5 > ")); - CHECK(!rc.parse(" 5 >= ")); + CHECK(false == rc.parse(" 5 = ")); + CHECK(false == rc.parse(" 5 ! ")); + CHECK(false == rc.parse(" 5 != ")); + CHECK(false == rc.parse(" 5 < ")); + CHECK(false == rc.parse(" 5 <= ")); + CHECK(false == rc.parse(" 5 > ")); + CHECK(false == rc.parse(" 5 >= ")); // missing bound - CHECK(!rc.parse(" 1 <> ")); - CHECK(!rc.parse(" <> 5 ")); - CHECK(!rc.parse(" 1 <=> ")); - CHECK(!rc.parse(" <=> 5 ")); + CHECK(false == rc.parse(" 1 <> ")); + CHECK(false == rc.parse(" <> 5 ")); + CHECK(false == rc.parse(" 1 <=> ")); + CHECK(false == rc.parse(" <=> 5 ")); } } @@ -611,25 +611,25 @@ TEST_CASE("validate", "[RangeCheck]") { RangeCheck rc; - REQUIRE(rc.validate("2<>4", "0:10")); + REQUIRE(true == rc.validate("2<>4", "0:10")); CHECK((rc.min == 2)); CHECK((rc.max == 4)); // # - CHECK(rc.validate("2<>4", "0")); + CHECK(true == rc.validate("2<>4", "0")); // #: - CHECK(rc.validate("2<>4", "1:")); + CHECK(true == rc.validate("2<>4", "1:")); // :# - CHECK(rc.validate("2<>4", ":8")); + CHECK(true == rc.validate("2<>4", ":8")); // in hex - CHECK(rc.validate("2<>4", "0x1:0x0A")); + CHECK(true == rc.validate("2<>4", "0x1:0x0A")); // invalid low - CHECK(!rc.validate("2<>4", "3:")); + CHECK(false == rc.validate("2<>4", "3:")); // invalid hi - CHECK(!rc.validate("2<>4", "1:3")); + CHECK(false == rc.validate("2<>4", "1:3")); // invalid low and hi - CHECK(!rc.validate("200<>400", "3:10")); + CHECK(false == rc.validate("200<>400", "3:10")); } #endif diff --git a/src/framework/test/data_bus_test.cc b/src/framework/test/data_bus_test.cc index 7de08bf09..ab5806ec4 100644 --- a/src/framework/test/data_bus_test.cc +++ b/src/framework/test/data_bus_test.cc @@ -35,7 +35,7 @@ using namespace snort; //-------------------------------------------------------------------------- // mocks //-------------------------------------------------------------------------- -InspectionPolicy::InspectionPolicy(unsigned int) {} +InspectionPolicy::InspectionPolicy(unsigned int) : framework_policy(nullptr), cloned(false) {} InspectionPolicy::~InspectionPolicy() = default; NetworkPolicy::NetworkPolicy(unsigned int, unsigned int) {} NetworkPolicy::~NetworkPolicy() = default; diff --git a/src/framework/value.cc b/src/framework/value.cc index be36de7fd..d4d21fef9 100644 --- a/src/framework/value.cc +++ b/src/framework/value.cc @@ -265,8 +265,8 @@ std::string Value::get_origin_string() const std::string value; std::string token; - stringstream s(origin_str); - while ( s >> token ) + stringstream str_s(origin_str); + while ( str_s >> token ) { value += token; value += " "; @@ -332,18 +332,40 @@ void Value::update_mask(uint64_t& mask, uint64_t flag, bool invert) // and the string value/zero is returned based on the result etc. TEST_CASE("set double test", "[Value]") +{ + Value test_val((double)12345.689); + CHECK(true == test_val.get_bool()); + CHECK(12345 == test_val.get_size()); + CHECK(12345 == test_val.get_int16()); + CHECK(12345 == test_val.get_uint16()); + CHECK(12345 == test_val.get_int32()); + CHECK(12345 == test_val.get_uint32()); + CHECK(12345 == test_val.get_int64()); + CHECK(12345 == test_val.get_uint64()); + CHECK(12345.689 == test_val.get_real()); + CHECK(12345 == test_val.get_ip4()); +} + +TEST_CASE("set double test 2", "[Value]") { Value test_val((double)123456.89); CHECK(true == test_val.get_bool()); CHECK(123456 == test_val.get_size()); - CHECK(-7616 == test_val.get_int16()); - CHECK(0xe240 == test_val.get_uint16()); - CHECK(0x1e240 == test_val.get_int32()); - CHECK(0x1e240 == test_val.get_uint32()); - CHECK(0x1e240 == test_val.get_int64()); - CHECK(0x1e240 == test_val.get_uint64()); + CHECK(123456 == test_val.get_int32()); + CHECK(123456 == test_val.get_uint32()); + CHECK(123456 == test_val.get_int64()); + CHECK(123456 == test_val.get_uint64()); CHECK(123456.89 == test_val.get_real()); - CHECK(0x1e240 == test_val.get_ip4()); + CHECK(123456 == test_val.get_ip4()); +} + +TEST_CASE("set double test 3", "[Value]") +{ + Value test_val((double)-123456.89); + CHECK(true == test_val.get_bool()); + CHECK(-123456 == test_val.get_int32()); + CHECK(-123456 == test_val.get_int64()); + CHECK(-123456.89 == test_val.get_real()); } TEST_CASE("mac addr negative test", "[Value]") @@ -462,7 +484,7 @@ TEST_CASE("token test", "[Value]") CHECK(true == test_val.get_next_csv_token(test_str)); str_val = (const char *)test_str.c_str(); REQUIRE(nullptr != str_val); - CHECK((strcmp(str_val,"123456")==0)); + CHECK(0 == strcmp(str_val,"123456")); } TEST_CASE("get as string", "[Value]") diff --git a/src/hash/lru_cache_local.h b/src/hash/lru_cache_local.h index 4cc87f1e2..8d0f38237 100644 --- a/src/hash/lru_cache_local.h +++ b/src/hash/lru_cache_local.h @@ -129,7 +129,7 @@ void LruCacheLocal::add_entry(const Key& key, const Value& val template Value& LruCacheLocal::find_else_create(const Key& key, bool* is_new) { - LruMapIter it = map.find(key); + auto it = map.find(key); if (it == map.end()) { stats.cache_misses++; @@ -147,7 +147,7 @@ Value& LruCacheLocal::find_else_create(const Key& key, bool* i template bool LruCacheLocal::add(const Key& key, const Value& value, bool replace) { - LruMapIter it = map.find(key); + auto it = map.find(key); if (it == map.end()) { stats.cache_misses++; @@ -168,7 +168,7 @@ bool LruCacheLocal::add(const Key& key, const Value& value, bo template bool LruCacheLocal::remove(const Key& key) { - LruMapIter it = map.find(key); + auto it = map.find(key); if (it == map.end()) { return false; diff --git a/src/hash/lru_cache_shared.h b/src/hash/lru_cache_shared.h index a476e45ff..4460ee266 100644 --- a/src/hash/lru_cache_shared.h +++ b/src/hash/lru_cache_shared.h @@ -79,22 +79,23 @@ public: using KeyType = Key; // Return data entry associated with key. If doesn't exist, return nullptr. - Data find(const Key& key); + Data find(const Key&); // Return data entry associated with key. If doesn't exist, create a new entry. - Data operator[](const Key& key); + Data operator[](const Key&); // Same as operator[]; additionally, sets the boolean if a new entry is created. - Data find_else_create(const Key& key, bool* new_data); + Data find_else_create(const Key&, bool* new_data); // Returns true if found or replaced, takes a ref to a user managed entry - bool find_else_insert(const Key& key, std::shared_ptr& data, bool replace = false); + bool find_else_insert(const Key&, Data&, bool replace = false); // Returns the found or inserted data, takes a ref to user managed entry. - Data find_else_insert(const Key&, Data&, LcsInsertStatus*, bool = false); + Data find_else_insert(const Key&, Data&, LcsInsertStatus*, + bool replace = false); // Return all data from the LruCache in order (most recently used to least) - std::vector > get_all_data(); + std::vector> get_all_data(); // Get current number of elements in the LruCache. size_t size() @@ -109,7 +110,7 @@ public: return list.size() * mem_chunk; } - size_t get_max_size() + size_t get_max_size() const { return max_size; } @@ -120,18 +121,18 @@ public: // Remove entry associated with Key. // Returns true if entry existed, false otherwise. - virtual bool remove(const Key& key); + virtual bool remove(const Key&); // Remove entry associated with key and return removed data. // Returns true and copy of data if entry existed. Returns false if // entry did not exist. - virtual bool remove(const Key& key, Data& data); + virtual bool remove(const Key&, Data&); const PegInfo* get_pegs() const { return lru_cache_shared_peg_names; } - PegCount* get_counts() - { return (PegCount*)&stats; } + const PegCount* get_counts() const + { return (const PegCount*)&stats; } void lock() { cache_mutex.lock(); } @@ -218,10 +219,9 @@ bool LruCacheShared::set_max_size(size_t newsiz template std::shared_ptr LruCacheShared::find(const Key& key) { - LruMapIter map_iter; std::lock_guard cache_lock(cache_mutex); - map_iter = map.find(key); + auto map_iter = map.find(key); if (map_iter == map.end()) { stats.find_misses++; @@ -241,11 +241,9 @@ std::shared_ptr LruCacheShared::operator } template -std::shared_ptr LruCacheShared:: -find_else_create(const Key& key, bool* new_data) +std::shared_ptr LruCacheShared::find_else_create(const Key& key, + bool* new_data) { - LruMapIter map_iter; - // As with remove and operator[], we need a temporary list of references // to delay the destruction of the items being removed by prune(). // This is one instance where we cannot get by with directly locking and @@ -256,7 +254,7 @@ find_else_create(const Key& key, bool* new_data) std::lock_guard cache_lock(cache_mutex); - map_iter = map.find(key); + auto map_iter = map.find(key); if (map_iter != map.end()) { stats.find_hits++; @@ -283,15 +281,14 @@ find_else_create(const Key& key, bool* new_data) } template -bool LruCacheShared:: -find_else_insert(const Key& key, std::shared_ptr& data, bool replace) +bool LruCacheShared::find_else_insert(const Key& key, Data& data, + bool replace) { - LruMapIter map_iter; - Purgatory tmp_data; + std::lock_guard cache_lock(cache_mutex); - map_iter = map.find(key); + auto map_iter = map.find(key); if (map_iter != map.end()) { stats.find_hits++; @@ -324,15 +321,14 @@ find_else_insert(const Key& key, std::shared_ptr& data, bool replace) } template -std::shared_ptr LruCacheShared:: -find_else_insert(const Key& key, std::shared_ptr& data, LcsInsertStatus* status, bool replace) +std::shared_ptr LruCacheShared::find_else_insert(const Key& key, Data& data, + LcsInsertStatus* status, bool replace) { - LruMapIter map_iter; - Purgatory tmp_data; + std::lock_guard cache_lock(cache_mutex); - map_iter = map.find(key); + auto map_iter = map.find(key); if (map_iter != map.end()) { stats.find_hits++; @@ -368,25 +364,19 @@ find_else_insert(const Key& key, std::shared_ptr& data, LcsInsertStatus* } template -std::vector< std::pair> > -LruCacheShared::get_all_data() +std::vector>> LruCacheShared::get_all_data() { std::vector > vec; std::lock_guard cache_lock(cache_mutex); - for (auto& entry : list ) - { - vec.emplace_back(entry); - } - + vec.reserve(list.size()); + std::copy(list.cbegin(), list.cend(), std::back_inserter(vec)); return vec; } template bool LruCacheShared::remove(const Key& key) { - LruMapIter map_iter; - // There is a potential race condition here, when the destructor of // the object being removed needs to call back into the cache and lock // the cache (e.g. via an allocator) to update the size of the cache. @@ -402,11 +392,9 @@ bool LruCacheShared::remove(const Key& key) std::lock_guard cache_lock(cache_mutex); - map_iter = map.find(key); + auto map_iter = map.find(key); if (map_iter == map.end()) - { return false; // Key is not in LruCache. - } data = map_iter->second->second; @@ -424,18 +412,13 @@ bool LruCacheShared::remove(const Key& key) } template -bool LruCacheShared::remove(const Key& key, - std::shared_ptr& data) +bool LruCacheShared::remove(const Key& key, Data& data) { - LruMapIter map_iter; - std::lock_guard cache_lock(cache_mutex); - map_iter = map.find(key); + auto map_iter = map.find(key); if (map_iter == map.end()) - { return false; // Key is not in LruCache. - } data = map_iter->second->second; diff --git a/src/hash/lru_segmented_cache_shared.h b/src/hash/lru_segmented_cache_shared.h index c6d4f96ae..1d744ee27 100644 --- a/src/hash/lru_segmented_cache_shared.h +++ b/src/hash/lru_segmented_cache_shared.h @@ -20,7 +20,9 @@ #ifndef LRU_SEGMENTED_CACHE_SHARED_H #define LRU_SEGMENTED_CACHE_SHARED_H +#include #include +#include #include #include "lru_cache_shared.h" @@ -28,7 +30,7 @@ #define DEFAULT_SEGMENT_COUNT 4 template, typename Eq = std::equal_to> -class SegmentedLruCache +class SegmentedLruCache { public: @@ -66,10 +68,10 @@ public: std::size_t segment_idx = get_segment_idx(key); return segments[segment_idx]->remove(key); } - + bool remove(const Key& key, Data& data) { - std::size_t idx = get_segment_idx(key); + std::size_t idx = get_segment_idx(key); return segments[idx]->remove(key, data); } @@ -90,7 +92,7 @@ public: std::size_t segment_idx = get_segment_idx(key); return segments[segment_idx]->find_else_insert(key, data, status, replace); } - + bool set_max_size(size_t max_size) { bool success = true; @@ -115,22 +117,20 @@ public: return all_data; } - size_t mem_size() + size_t mem_size() const { size_t mem_size = 0; for ( const auto& cache : segments ) - { mem_size += cache->mem_size(); - } return mem_size; } - const PegInfo* get_pegs() - { - return lru_cache_shared_peg_names; + const PegInfo* get_pegs() + { + return lru_cache_shared_peg_names; } - PegCount* get_counts() + const PegCount* get_counts() { PegCount* pcs = (PegCount*)&counts; const PegInfo* pegs = get_pegs(); @@ -144,26 +144,22 @@ public: } pcs[i] = c; } - return (PegCount*)&counts; + return (const PegCount*)&counts; } - size_t size() + size_t size() const { size_t total_size = 0; - for ( const auto& cache : segments ) - { + for ( const auto& cache : segments ) total_size += cache->size(); - } return total_size; } - size_t get_max_size() + size_t get_max_size() const { size_t max_size = 0; for ( const auto& cache : segments ) - { max_size += cache->get_max_size(); - } return max_size; } diff --git a/src/hash/test/ghash_test.cc b/src/hash/test/ghash_test.cc index ea0b150cb..79e8d7a5e 100644 --- a/src/hash/test/ghash_test.cc +++ b/src/hash/test/ghash_test.cc @@ -42,7 +42,7 @@ DataBus::DataBus() = default; DataBus::~DataBus() = default; // run_flags is used indirectly from HashFnc class by calling SnortConfig::static_hash() -SnortConfig::SnortConfig(const SnortConfig* const, const char*) +SnortConfig::SnortConfig(const SnortConfig* const, const char*) : daq_config(nullptr), thread_config(nullptr) { snort_conf->run_flags = 0;} SnortConfig::~SnortConfig() = default; diff --git a/src/hash/test/lru_cache_shared_test.cc b/src/hash/test/lru_cache_shared_test.cc index 360858500..115836a5e 100644 --- a/src/hash/test/lru_cache_shared_test.cc +++ b/src/hash/test/lru_cache_shared_test.cc @@ -191,7 +191,7 @@ TEST(lru_cache_shared, stats_test) lru_cache.remove(7); // Removes - hit lru_cache.remove(10); // Removes - miss - PegCount* stats = lru_cache.get_counts(); + const PegCount* stats = lru_cache.get_counts(); CHECK(stats[0] == 10); // adds CHECK(stats[1] == 7); // alloc prunes diff --git a/src/hash/test/lru_seg_cache_shared_test.cc b/src/hash/test/lru_seg_cache_shared_test.cc index 9779e9e07..e703f22c0 100644 --- a/src/hash/test/lru_seg_cache_shared_test.cc +++ b/src/hash/test/lru_seg_cache_shared_test.cc @@ -179,7 +179,7 @@ TEST(segmented_lru_cache, stats_test) lru_cache.remove(8); lru_cache.remove(11); // not in cache - PegCount* stats = lru_cache.get_counts(); + const PegCount* stats = lru_cache.get_counts(); CHECK(stats[0] == 10); // adds CHECK(stats[1] == 2); // alloc_prunes @@ -199,10 +199,10 @@ TEST(segmented_lru_cache, find_else_insert_replace) SegmentedLruCache lru_cache(8); std::shared_ptr data(new std::string("hello")); LcsInsertStatus status; - + lru_cache.find_else_insert(1, data, &status, false); // initial insert CHECK(status == LcsInsertStatus::LCS_ITEM_INSERTED); // Check status for initial insert - + std::shared_ptr newData(new std::string("world")); std::shared_ptr returnedData = lru_cache.find_else_insert(1, newData, &status, true); // replace existing item CHECK(returnedData != nullptr); diff --git a/src/hash/test/xhash_test.cc b/src/hash/test/xhash_test.cc index 112d94128..1e3ba74ba 100644 --- a/src/hash/test/xhash_test.cc +++ b/src/hash/test/xhash_test.cc @@ -42,7 +42,7 @@ DataBus::DataBus() = default; DataBus::~DataBus() = default; // run_flags is used indirectly from HashFnc class by calling SnortConfig::static_hash() -SnortConfig::SnortConfig(const SnortConfig* const, const char*) +SnortConfig::SnortConfig(const SnortConfig* const, const char*) : daq_config(nullptr), thread_config(nullptr) { snort_conf->run_flags = 0;} SnortConfig::~SnortConfig() = default; diff --git a/src/hash/test/zhash_test.cc b/src/hash/test/zhash_test.cc index ed5ddd47b..551cfb899 100644 --- a/src/hash/test/zhash_test.cc +++ b/src/hash/test/zhash_test.cc @@ -67,7 +67,7 @@ DataBus::DataBus() = default; DataBus::~DataBus() = default; // run_flags is used indirectly from HashFnc class by calling SnortConfig::static_hash() -SnortConfig::SnortConfig(const SnortConfig* const, const char*) +SnortConfig::SnortConfig(const SnortConfig* const, const char*) : daq_config(nullptr), thread_config(nullptr) { snort_conf->run_flags = 0;} SnortConfig::~SnortConfig() = default; diff --git a/src/helpers/discovery_filter.cc b/src/helpers/discovery_filter.cc index 2cb7394f2..b04218002 100644 --- a/src/helpers/discovery_filter.cc +++ b/src/helpers/discovery_filter.cc @@ -179,7 +179,7 @@ DiscoveryFilter::DiscoveryFilter(const string& conf_path) auto any_list = get_list((FilterType)type, DF_ANY_INTF); if (!any_list) continue; - for (auto& intf_entry : intf_ip_list[type]) + for (const auto& intf_entry : intf_ip_list[type]) { if (intf_entry.second != any_list and sfvar_add(intf_entry.second, any_list) != SFIP_SUCCESS) @@ -412,27 +412,27 @@ TEST_CASE("Discovery Filter", "[is_monitored]") DiscoveryFilter df(conf); // Without flag - CHECK(df.is_app_monitored(&p, nullptr) == true); // any interface rule for app is added to interface 0 - CHECK(df.is_host_monitored(&p, nullptr) == false); // no rule for host - CHECK(df.is_user_monitored(&p, nullptr) == false); // no any interface rule for user + CHECK(true == df.is_app_monitored(&p, nullptr)); // any interface rule for app is added to interface 0 + CHECK(false == df.is_host_monitored(&p, nullptr)); // no rule for host + CHECK(false == df.is_user_monitored(&p, nullptr)); // no any interface rule for user // With flag uint8_t flag = 0; CHECK((flag & DF_APP_CHECKED) != DF_APP_CHECKED); CHECK((flag & DF_APP_MONITORED) != DF_APP_MONITORED); - CHECK(df.is_app_monitored(&p, &flag) == true); // first attempt + CHECK(true == df.is_app_monitored(&p, &flag)); // first attempt CHECK((flag & DF_APP_CHECKED) == DF_APP_CHECKED); CHECK((flag & DF_APP_MONITORED) == DF_APP_MONITORED); - CHECK(df.is_app_monitored(&p, &flag) == true); // second attempt + CHECK(true == df.is_app_monitored(&p, &flag)); // second attempt CHECK((flag & DF_APP_CHECKED) == DF_APP_CHECKED); CHECK((flag & DF_APP_MONITORED) == DF_APP_MONITORED); CHECK((flag & DF_USER_CHECKED) != DF_USER_CHECKED); CHECK((flag & DF_USER_MONITORED) != DF_USER_MONITORED); - CHECK(df.is_user_monitored(&p, &flag) == false); // first attempt + CHECK(false == df.is_user_monitored(&p, &flag)); // first attempt CHECK((flag & DF_USER_CHECKED) == DF_USER_CHECKED); CHECK((flag & DF_USER_MONITORED) != DF_USER_MONITORED); - CHECK(df.is_user_monitored(&p, &flag) == false); // second attempt + CHECK(false == df.is_user_monitored(&p, &flag)); // second attempt CHECK((flag & DF_USER_CHECKED) == DF_USER_CHECKED); CHECK((flag & DF_USER_MONITORED) != DF_USER_MONITORED); @@ -453,9 +453,9 @@ TEST_CASE("Discovery Filter Empty Configuration", "[is_monitored_config]") p.ptrs.ip_api.set(ip, ip); DiscoveryFilter df(conf); - CHECK(df.is_app_monitored(&p, nullptr) == false); - CHECK(df.is_host_monitored(&p, nullptr) == false); - CHECK(df.is_user_monitored(&p, nullptr) == false); + CHECK(false == df.is_app_monitored(&p, nullptr)); + CHECK(false == df.is_host_monitored(&p, nullptr)); + CHECK(false == df.is_user_monitored(&p, nullptr)); remove("test_empty_analyze.txt"); } @@ -490,33 +490,33 @@ TEST_CASE("Discovery Filter Intf", "[is_monitored_intf_vs_ip]") p.ptrs.ip_api.set(ip1, ip7); // ip from undefined interface matches interface any list p.pkth = &z_undefined; - CHECK(df.is_app_monitored(&p, nullptr) == true); // analyze host enables application discovery - CHECK(df.is_host_monitored(&p, nullptr) == true); - CHECK(df.is_user_monitored(&p, nullptr) == false); + CHECK(true == df.is_app_monitored(&p, nullptr)); // analyze host enables application discovery + CHECK(true == df.is_host_monitored(&p, nullptr)); + CHECK(false == df.is_user_monitored(&p, nullptr)); p.pkth = &z2; // the ip is not in interface 2 list, but it is in interface any list - CHECK(df.is_host_monitored(&p, nullptr) == true); + CHECK(true == df.is_host_monitored(&p, nullptr)); p.ptrs.ip_api.set(ip3, ip7); // the ip matches interface 2 list - CHECK(df.is_host_monitored(&p, nullptr) == true); + CHECK(true == df.is_host_monitored(&p, nullptr)); p.pkth = &z1; // no interface 1 list and the ip is not in interface any list - CHECK(df.is_host_monitored(&p, nullptr) == false); + CHECK(false == df.is_host_monitored(&p, nullptr)); p.ptrs.ip_api.set(ip1, ip7); // no interface 1 list, but the ip is in interface any list - CHECK(df.is_host_monitored(&p, nullptr) == true); + CHECK(true == df.is_host_monitored(&p, nullptr)); p.pkth = saved_hdr; p.ptrs.ip_api.set(ip2, ip7); // the ip matches interface 0 list - CHECK(df.is_host_monitored(&p, nullptr) == true); + CHECK(true == df.is_host_monitored(&p, nullptr)); // no match since the configuration for these ip addresses were invalid p.ptrs.ip_api.set(ip4, ip7); - CHECK(df.is_host_monitored(&p, nullptr) == false); + CHECK(false == df.is_host_monitored(&p, nullptr)); p.ptrs.ip_api.set(ip5, ip7); - CHECK(df.is_host_monitored(&p, nullptr) == false); + CHECK(false == df.is_host_monitored(&p, nullptr)); p.ptrs.ip_api.set(ip6, ip7); - CHECK(df.is_host_monitored(&p, nullptr) == false); + CHECK(false == df.is_host_monitored(&p, nullptr)); remove("test_intf_ip.txt"); } @@ -572,7 +572,7 @@ TEST_CASE("Discovery Filter Port Exclusion", "[portexclusion]") p.ptrs.dp = b_port; p.packet_flags = 0x0; p.packet_flags |= PKT_FROM_CLIENT; - CHECK(is_port_excluded_test(df, &p) == true); + CHECK(true == is_port_excluded_test(df, &p)); // A:any <- B:b_port p.ptrs.ip_api.set(&ba_hdr); @@ -580,7 +580,7 @@ TEST_CASE("Discovery Filter Port Exclusion", "[portexclusion]") p.ptrs.dp = a_port; p.packet_flags = 0x0; p.packet_flags |= PKT_FROM_SERVER; - CHECK(is_port_excluded_test(df, &p) == true); + CHECK(true == is_port_excluded_test(df, &p)); // Negative test: B = initiator (client), A = responder (server) @@ -592,7 +592,7 @@ TEST_CASE("Discovery Filter Port Exclusion", "[portexclusion]") p.ptrs.dp = a_port; p.packet_flags = 0x0; p.packet_flags |= PKT_FROM_CLIENT; - CHECK(is_port_excluded_test(df, &p) == false); + CHECK(false == is_port_excluded_test(df, &p)); // A -> B:b_port p.ptrs.ip_api.set(&ab_hdr); @@ -600,7 +600,7 @@ TEST_CASE("Discovery Filter Port Exclusion", "[portexclusion]") p.ptrs.dp = b_port; p.packet_flags = 0x0; p.packet_flags |= PKT_FROM_SERVER; - CHECK(is_port_excluded_test(df, &p) == false); + CHECK(false == is_port_excluded_test(df, &p)); remove(conf.c_str()); } diff --git a/src/helpers/sigsafe.cc b/src/helpers/sigsafe.cc index d054f068b..afb992431 100644 --- a/src/helpers/sigsafe.cc +++ b/src/helpers/sigsafe.cc @@ -391,6 +391,7 @@ TEST_CASE("sigsafe printer", "[SigsafePrinter]") SECTION("null string") { const char* nullstr = nullptr; + // cppcheck-suppress nullPointer snprintf(expected, sizeof(expected), "%s", nullstr); SigSafePrinter(actual, sizeof(actual)).printf("%s", nullstr); CHECK_THAT(expected, Equals(actual)); diff --git a/src/helpers/test/bitop_test.cc b/src/helpers/test/bitop_test.cc index c9282f67c..f3303716a 100644 --- a/src/helpers/test/bitop_test.cc +++ b/src/helpers/test/bitop_test.cc @@ -55,12 +55,12 @@ TEST_CASE( "bitop", "[bitop]" ) const size_t bit = 7; bitop.set(bit); - CHECK(bitop.is_set(bit)); + CHECK(true == bitop.is_set(bit)); CHECK(num_set(bitop, max) == 1); bitop.clear(bit); - CHECK(!bitop.is_set(bit)); + CHECK(false == bitop.is_set(bit)); CHECK( (is_clear(bitop, max) == true) ); } @@ -71,23 +71,23 @@ TEST_CASE( "bitop", "[bitop]" ) const size_t k = max + 2; bitop.set(j); - CHECK(bitop.is_set(j)); + CHECK(true == bitop.is_set(j)); - CHECK(!bitop.is_set(k)); + CHECK(false == bitop.is_set(k)); bitop.set(k); - CHECK(bitop.is_set(k)); + CHECK(true == bitop.is_set(k)); CHECK(num_set(bitop, k + 2) == 2); - CHECK(bitop.is_set(j)); + CHECK(true == bitop.is_set(j)); bitop.clear(k); - CHECK(!bitop.is_set(k)); + CHECK(false == bitop.is_set(k)); - CHECK(bitop.is_set(j)); + CHECK(true == bitop.is_set(j)); bitop.clear(j); - CHECK( (is_clear(bitop, k + 2) == true) ); + CHECK(true == is_clear(bitop, k + 2)); } } diff --git a/src/host_tracker/host_cache_module.cc b/src/host_tracker/host_cache_module.cc index f211fbd39..4b19e505a 100644 --- a/src/host_tracker/host_cache_module.cc +++ b/src/host_tracker/host_cache_module.cc @@ -365,7 +365,7 @@ static const Parameter host_cache_params[] = { "memcap", Parameter::PT_INT, "512:maxSZ", "8388608", "maximum host cache size in bytes" }, - + { "segments", Parameter::PT_INT, "1:32", "4", "number of host cache segments. It must be power of 2."}, @@ -385,7 +385,7 @@ bool HostCacheModule::set(const char*, Value& v, SnortConfig*) else if ( v.is("segments")) { segments = v.get_uint8(); - + if(segments > 32) segments = 32; @@ -408,7 +408,7 @@ bool HostCacheModule::end(const char* fqn, int, SnortConfig* sc) if ( Snort::is_reloading() ) sc->register_reload_handler(new HostCacheReloadTuner(memcap)); else - { + { host_cache.setup(segments, memcap); ControlConn::log_command("host_cache.delete_host",false); } @@ -485,7 +485,7 @@ string HostCacheModule::get_host_cache_segment_stats(int seg_idx) if(seg_idx >= host_cache.get_segments()) return "Invalid segment index\nTry host_cache.get_segment_stats() to get all stats\n"; - + string str; const PegInfo* pegs = host_cache.get_pegs(); @@ -503,7 +503,7 @@ string HostCacheModule::get_host_cache_segment_stats(int seg_idx) cache->stats.items_in_use = cache->list.size(); cache->unlock(); } - + PegCount* counts = (PegCount*) host_cache.get_counts(); for ( int i = 0; pegs[i].type != CountType::END; i++ ) { @@ -536,7 +536,7 @@ string HostCacheModule::get_host_cache_segment_stats(int seg_idx) cache->stats.items_in_use = cache->list.size(); cache->unlock(); - PegCount* count = (PegCount*) cache->get_counts(); + const PegCount* count = cache->get_counts(); for ( int i = 0; pegs[i].type != CountType::END; i++ ) { if ( count[i] ) @@ -566,7 +566,7 @@ string HostCacheModule::get_host_cache_stats() cache->stats.items_in_use = cache->list.size(); cache->unlock(); } - + PegCount* counts = (PegCount*) host_cache.get_counts(); const PegInfo* pegs = host_cache.get_pegs(); diff --git a/src/host_tracker/host_cache_segmented.h b/src/host_tracker/host_cache_segmented.h index e417435d5..e80519262 100644 --- a/src/host_tracker/host_cache_segmented.h +++ b/src/host_tracker/host_cache_segmented.h @@ -26,6 +26,7 @@ #include #include +#include #include "host_cache.h" #include "log/messages.h" @@ -38,8 +39,8 @@ template class HostCacheSegmented { public: - HostCacheSegmented() : - segment_count(DEFAULT_HOST_CACHE_SEGMENTS), + HostCacheSegmented() : + segment_count(DEFAULT_HOST_CACHE_SEGMENTS), memcap_per_segment(LRU_CACHE_INITIAL_SIZE) { } HostCacheSegmented(uint8_t segment_count, size_t memcap_per_segment); @@ -93,7 +94,7 @@ HostCacheSegmented::HostCacheSegmented(uint8_t segment_count, size_t memcap_per_segment(memcap_per_segment) { assert(segment_count > 0); - + for (size_t i = 0; i < this->segment_count; ++i) { auto cache = new HostCacheIp(this->memcap_per_segment); @@ -109,7 +110,7 @@ void HostCacheSegmented::init() return; assert(segment_count > 0); - + for (size_t i = 0; i < segment_count; ++i) { auto cache = new HostCacheIp(memcap_per_segment.load()); @@ -139,16 +140,16 @@ void HostCacheSegmented::setup(uint8_t segs, size_t memcap ) } template -size_t HostCacheSegmented::get_valid_id(uint8_t idx) -{ +size_t HostCacheSegmented::get_valid_id(uint8_t idx) +{ if(idx < seg_list.size()) return seg_list[idx]->get_valid_id(); return 0; } template -void HostCacheSegmented::print_config() -{ +void HostCacheSegmented::print_config() +{ if ( snort::SnortConfig::log_verbose() ) { snort::LogLabel("host_cache"); @@ -180,7 +181,7 @@ bool HostCacheSegmented::set_max_size(size_t max_size) } /** - * Resize the cache based on the provided memory capacity, distributing the + * Resize the cache based on the provided memory capacity, distributing the * memory equally among all the segments. If any segment fails to resize, * the operation is considered unsuccessful. */ @@ -199,11 +200,11 @@ bool HostCacheSegmented::reload_resize(size_t memcap) // Computes the index of the segment where a given key-value pair belongs. template -uint8_t HostCacheSegmented::get_segment_idx(Key val) +uint8_t HostCacheSegmented::get_segment_idx(Key val) { const uint8_t* bytes = reinterpret_cast(&val); uint8_t result = 0; - for (size_t i = 0; i < sizeof(Key); ++i) + for (size_t i = 0; i < sizeof(Key); ++i) result ^= bytes[i]; //Assumes segment_count is a power of 2 always //This is a fast way to do a modulo operation @@ -232,7 +233,7 @@ std::shared_ptr HostCacheSegmented::find(const Key& key) } /** - * Updates the internal counts of the host cache. This method aggregates the + * Updates the internal counts of the host cache. This method aggregates the * counts from all segments and updates the overall counts for the cache. */ template @@ -245,9 +246,7 @@ void HostCacheSegmented::update_counts() { PegCount c = 0; for(auto cache : seg_list) - { c += cache->get_counts()[i]; - } pcs[i] = c; } } @@ -258,7 +257,7 @@ std::shared_ptr HostCacheSegmented:: find_else_create(const K // Determine the segment index where the key-value pair resides or should reside uint8_t idx = get_segment_idx(key); bool new_data_local = false; - + // Retrieve or create the entry for the key in the determined segment auto ht = seg_list[idx]->find_else_create(key, &new_data_local); if(new_data_local) @@ -326,9 +325,7 @@ size_t HostCacheSegmented::get_max_size() { size_t max_size = 0; for (auto cache : seg_list) - { max_size += cache->get_max_size(); - } return max_size; } @@ -340,7 +337,7 @@ size_t HostCacheSegmented::get_mem_chunk() } template -bool HostCacheSegmented::remove(const Key& key) +bool HostCacheSegmented::remove(const Key& key) { uint8_t idx = get_segment_idx(key); return seg_list[idx]->remove(key); @@ -358,19 +355,19 @@ Warning!!!: update_allocator and update_set_allocator don't copy data to old con but erase it for speed. Use with care!!! */ template