From: Steve Chew (stechew) Date: Mon, 1 Jun 2020 18:33:07 +0000 (+0000) Subject: Merge pull request #2225 in SNORT/snort3 from ~SBAIGAL/snort3:coverity_fix2 to master X-Git-Tag: 3.0.1-5~39 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ae4624439f67e00cfea5d0033f18b80f8aed673b;p=thirdparty%2Fsnort3.git Merge pull request #2225 in SNORT/snort3 from ~SBAIGAL/snort3:coverity_fix2 to master Squashed commit of the following: commit 0d0ea620abf03e13f68e3ce714eefa26b2cb310b Author: Steven Baigal (sbaigal) Date: Thu May 14 10:09:44 2020 -0400 coverity: fix issued found by Coverity scans --- diff --git a/src/codecs/ip/cd_udp.cc b/src/codecs/ip/cd_udp.cc index e6745b41e..50125b644 100644 --- a/src/codecs/ip/cd_udp.cc +++ b/src/codecs/ip/cd_udp.cc @@ -117,13 +117,13 @@ public: bool is_vxlan_port(uint16_t port) { return vxlan_ports.test(port); } - void set_gtp_ports(PortBitSet ports) + void set_gtp_ports(const PortBitSet& ports) { gtp_ports = ports; gtp_decode = ports.any(); } - void set_vxlan_ports(PortBitSet ports) + void set_vxlan_ports(const PortBitSet& ports) { vxlan_ports = ports; vxlan_decode = ports.any(); diff --git a/src/connectors/tcp_connector/tcp_connector.h b/src/connectors/tcp_connector/tcp_connector.h index 2c0564432..9caf29883 100644 --- a/src/connectors/tcp_connector/tcp_connector.h +++ b/src/connectors/tcp_connector/tcp_connector.h @@ -81,7 +81,7 @@ public: int sock_fd; private: - bool run_thread; + bool run_thread = false; std::thread* receive_thread; void start_receive_thread(); void stop_receive_thread(); diff --git a/src/connectors/tcp_connector/tcp_connector_config.h b/src/connectors/tcp_connector/tcp_connector_config.h index f3c663c55..5c0cd56e9 100644 --- a/src/connectors/tcp_connector/tcp_connector_config.h +++ b/src/connectors/tcp_connector/tcp_connector_config.h @@ -32,9 +32,9 @@ public: TcpConnectorConfig() { direction = snort::Connector::CONN_DUPLEX; async_receive = true; } - uint16_t base_port; + uint16_t base_port = 0; std::string address; - Setup setup; + Setup setup = {}; bool async_receive; typedef std::vector TcpConnectorConfigSet; diff --git a/src/detection/fp_create.cc b/src/detection/fp_create.cc index c9e6bc96e..cca0034a1 100644 --- a/src/detection/fp_create.cc +++ b/src/detection/fp_create.cc @@ -174,7 +174,7 @@ static int otn_create_tree(OptTreeNode* otn, void** existing_tree, Mpse::MpseTyp child->num_children++; child->children = (detection_option_tree_node_t**) - snort_calloc(child->num_children, sizeof(child->children)); + snort_calloc(child->num_children, sizeof(detection_option_tree_node_t*)); child->is_relative = opt_fp->isRelative; if (node && child->is_relative) @@ -245,7 +245,7 @@ static int otn_create_tree(OptTreeNode* otn, void** existing_tree, Mpse::MpseTyp { node->num_children++; tmp_children = (detection_option_tree_node_t**) - snort_calloc(node->num_children, sizeof(tmp_children)); + snort_calloc(node->num_children, sizeof(detection_option_tree_node_t*)); memcpy(tmp_children, node->children, sizeof(detection_option_tree_node_t*) * (node->num_children-1)); @@ -289,7 +289,7 @@ static int otn_create_tree(OptTreeNode* otn, void** existing_tree, Mpse::MpseTyp detection_option_tree_node_t** tmp_children; root->num_children++; tmp_children = (detection_option_tree_node_t**) - snort_calloc(root->num_children, sizeof(tmp_children)); + snort_calloc(root->num_children, sizeof(detection_option_tree_node_t*)); memcpy(tmp_children, root->children, sizeof(detection_option_tree_node_t*) * (root->num_children-1)); snort_free(root->children); @@ -304,7 +304,7 @@ static int otn_create_tree(OptTreeNode* otn, void** existing_tree, Mpse::MpseTyp detection_option_tree_node_t** tmp_children; node->num_children++; tmp_children = (detection_option_tree_node_t**) - snort_calloc(node->num_children, sizeof(tmp_children)); + snort_calloc(node->num_children, sizeof(detection_option_tree_node_t*)); memcpy(tmp_children, node->children, sizeof(detection_option_tree_node_t*) * (node->num_children-1)); snort_free(node->children); @@ -419,8 +419,13 @@ static int fpFinishPortGroup(SnortConfig* sc, PortGroup* pg, FastPatternConfig* int i; int rules = 0; - if ((pg == nullptr) || (fp == nullptr)) + if (pg == nullptr) return -1; + if (fp == nullptr) + { + snort_free(pg); + return -1; + } for (i = PM_TYPE_PKT; i < PM_TYPE_MAX; i++) { diff --git a/src/detection/ips_context.h b/src/detection/ips_context.h index 18389b592..adcbc469b 100644 --- a/src/detection/ips_context.h +++ b/src/detection/ips_context.h @@ -140,23 +140,23 @@ public: std::vector rpl; Packet* packet; - Packet* wire_packet; + Packet* wire_packet = nullptr; Packet* encode_packet; DAQ_PktHdr_t* pkth; uint8_t* buf; - const SnortConfig* conf; + const SnortConfig* conf = nullptr; MpseBatch searches; MpseStash* stash; OtnxMatchData* otnx; std::list::iterator regex_req_it; SF_EVENTQ* equeue; - DataPointer file_data; - DataBuffer alt_data; + DataPointer file_data = {}; + DataBuffer alt_data = {}; uint64_t context_num; - uint64_t packet_number; + uint64_t packet_number = 0; ActiveRules active_rules; State state; bool check_tags; @@ -169,13 +169,13 @@ public: static constexpr unsigned max_ips_id = 8; private: - FlowSnapshot flow; + FlowSnapshot flow = {}; std::vector data; std::vector ids_in_use; // for indirection; FIXIT-P evaluate alternatives std::vector post_callbacks; IpsContext* depends_on; IpsContext* next_to_process; - bool remove_gadget; + bool remove_gadget = false; }; } #endif diff --git a/src/file_api/file_capture.cc b/src/file_api/file_capture.cc index ce0193c74..57a0f99d0 100644 --- a/src/file_api/file_capture.cc +++ b/src/file_api/file_capture.cc @@ -392,11 +392,6 @@ FileCaptureState FileCapture::reserve_file(const FileInfo* file) head = last = fileBlock; } - if (!fileBlock) - { - return error_capture(FILE_CAPTURE_MEMCAP); - } - /*Copy the last piece of file to file buffer*/ if (save_to_file_buffer(current_data, current_data_len, capture_max_size) ) diff --git a/src/file_api/file_flows.cc b/src/file_api/file_flows.cc index b35a6b5e2..76ac8f8db 100644 --- a/src/file_api/file_flows.cc +++ b/src/file_api/file_flows.cc @@ -97,14 +97,8 @@ FileFlows* FileFlows::get_file_flows(Flow* flow) { fd = new FileFlows(flow, fi); flow->set_flow_data(fd); - } - else - return fd; - - FileConfig* fc = fi->config; - if (fc and fd) - { - fd->set_file_policy(&(fc->get_file_policy())); + if (fi->config) + fd->set_file_policy(&(fi->config->get_file_policy())); } return fd; @@ -294,8 +288,7 @@ bool FileFlows::file_process(Packet* p, uint64_t file_id, const uint8_t* file_da if ((context->get_file_sig_sha256()) || !context->is_file_signature_enabled()) { /* Just check file type and signature */ - FilePosition position = SNORT_FILE_FULL; - continue_processing = context->process(p, file_data, data_size, position, + continue_processing = context->process(p, file_data, data_size, SNORT_FILE_FULL, file_policy); if (context->processing_complete) remove_processed_file_context(multi_file_processing_id); diff --git a/src/file_api/file_log.cc b/src/file_api/file_log.cc index 8c66a9c55..333931541 100644 --- a/src/file_api/file_log.cc +++ b/src/file_api/file_log.cc @@ -190,7 +190,7 @@ void LogHandler::handle(DataEvent&, Flow* f) uint64_t fsize = file->get_file_size(); if ( fsize > 0) - TextLog_Print(tlog, "[Size: %u] ", fsize); + TextLog_Print(tlog, "[Size: %lu] ", fsize); TextLog_Print(tlog, "\n"); diff --git a/src/file_api/file_mempool.h b/src/file_api/file_mempool.h index 9a2f04c39..6a689af41 100644 --- a/src/file_api/file_mempool.h +++ b/src/file_api/file_mempool.h @@ -72,11 +72,11 @@ private: void free_pools(); int remove(CircularBuffer* cb, void* obj); - void** datapool; /* memory buffer */ - uint64_t total; - CircularBuffer* free_list; - CircularBuffer* released_list; - size_t obj_size; + void** datapool = nullptr; /* memory buffer */ + uint64_t total = 0; + CircularBuffer* free_list = nullptr; + CircularBuffer* released_list = nullptr; + size_t obj_size = 0; std::mutex pool_mutex; }; diff --git a/src/framework/codec.h b/src/framework/codec.h index ec11a9a92..75a72c8d2 100644 --- a/src/framework/codec.h +++ b/src/framework/codec.h @@ -124,26 +124,23 @@ struct CodecData /* This section will get reset before every decode() function call */ ProtocolId next_prot_id; /* protocol type of the next layer */ - uint16_t lyr_len; /* The length of the valid part layer */ - uint16_t invalid_bytes; /* the length of the INVALID part of this layer */ + uint16_t lyr_len = 0; /* The length of the valid part layer */ + uint16_t invalid_bytes = 0; /* the length of the INVALID part of this layer */ /* Reset before each decode of packet begins */ /* Codec specific fields. These fields are only relevant to codecs. */ - uint32_t proto_bits; /* protocols contained within this packet + uint32_t proto_bits = 0; /* protocols contained within this packet -- will be propogated to Snort++ Packet struct*/ - uint16_t codec_flags; /* flags used while decoding */ - uint8_t ip_layer_cnt; - - /* The following values have junk values after initialization */ - uint8_t ip6_extension_count; /* initialized in cd_ipv6.cc */ - uint8_t curr_ip6_extension; /* initialized in cd_ipv6.cc */ - IpProtocol ip6_csum_proto; /* initialized in cd_ipv6.cc. Used for IPv6 checksums */ - bool tunnel_bypass; - - CodecData(const SnortConfig* sc, ProtocolId init_prot) : - conf(sc), next_prot_id(init_prot), lyr_len(0), invalid_bytes(0), - proto_bits(0), codec_flags(0), ip_layer_cnt(0), tunnel_bypass(false) + uint16_t codec_flags = 0; /* flags used while decoding */ + uint8_t ip_layer_cnt = 0; + + uint8_t ip6_extension_count = 0; + uint8_t curr_ip6_extension = 0; + IpProtocol ip6_csum_proto = IpProtocol::IP; /* Used for IPv6 checksums */ + bool tunnel_bypass = false; + + CodecData(const SnortConfig* sc, ProtocolId init_prot) : conf(sc), next_prot_id(init_prot) { } bool inline is_cooked() const diff --git a/src/framework/inspector.h b/src/framework/inspector.h index 0e64f9534..5189b010e 100644 --- a/src/framework/inspector.h +++ b/src/framework/inspector.h @@ -152,9 +152,9 @@ protected: Inspector(); // internal init only at this point private: - const InspectApi* api; + const InspectApi* api = nullptr; std::atomic_uint* ref_count; - SnortProtocolId snort_protocol_id; + SnortProtocolId snort_protocol_id = 0; }; // at present there is no sequencing among like types except that appid diff --git a/src/mime/file_mime_log.h b/src/mime/file_mime_log.h index 255a74321..f507be9f0 100644 --- a/src/mime/file_mime_log.h +++ b/src/mime/file_mime_log.h @@ -70,8 +70,8 @@ public: private: int log_flags = 0; uint8_t* buf = nullptr; - unsigned char* emailHdrs; - uint32_t log_depth; + unsigned char* emailHdrs = nullptr; + uint32_t log_depth = 0; uint32_t hdrs_logged; uint8_t* recipients = nullptr; uint16_t rcpts_logged; diff --git a/src/packet_io/active.cc b/src/packet_io/active.cc index 883434901..61da692cb 100644 --- a/src/packet_io/active.cc +++ b/src/packet_io/active.cc @@ -221,7 +221,8 @@ void Active::send_reset(Packet* p, EncodeFlags ef) if ( (p->packet_flags & PKT_USE_DIRECT_INJECT) or (p->flow and p->flow->flags.use_direct_inject) ) { - DIOCTL_DirectInjectReset msg = { p->daq_msg, !(ef & ENC_FLAG_FWD) }; + DIOCTL_DirectInjectReset msg = + { p->daq_msg, (uint8_t)((ef & ENC_FLAG_FWD) ? DAQ_DIR_FORWARD : DAQ_DIR_REVERSE) }; int ret = p->daq_instance->ioctl(DIOCTL_DIRECT_INJECT_RESET, &msg, sizeof(msg)); if ( ret != DAQ_SUCCESS ) @@ -296,7 +297,8 @@ uint32_t Active::send_data( EncodeFlags tmp_flags = flags ^ ENC_FLAG_FWD; if ( use_direct_inject ) { - DIOCTL_DirectInjectReset msg = { p->daq_msg, !(tmp_flags & ENC_FLAG_FWD) }; + DIOCTL_DirectInjectReset msg = + { p->daq_msg, (uint8_t)((tmp_flags & ENC_FLAG_FWD) ? DAQ_DIR_FORWARD : DAQ_DIR_REVERSE) }; ret = p->daq_instance->ioctl(DIOCTL_DIRECT_INJECT_RESET, &msg, sizeof(msg)); if ( ret != DAQ_SUCCESS ) @@ -334,7 +336,8 @@ uint32_t Active::send_data( flags = (flags & ~ENC_FLAG_VAL); const DAQ_DIPayloadSegment segments[] = { {buf, blen} }; const DAQ_DIPayloadSegment* payload[] = { &segments[0] }; - DIOCTL_DirectInjectPayload msg = { p->daq_msg, payload, 1, !(flags & ENC_FLAG_FWD)}; + DIOCTL_DirectInjectPayload msg = { p->daq_msg, payload, 1, + (uint8_t)((flags & ENC_FLAG_FWD) ? DAQ_DIR_FORWARD : DAQ_DIR_REVERSE) }; ret = p->daq_instance->ioctl(DIOCTL_DIRECT_INJECT_PAYLOAD, &msg, sizeof(msg)); if ( ret != DAQ_SUCCESS ) @@ -409,7 +412,8 @@ uint32_t Active::send_data( flags = (flags & ~ENC_FLAG_VAL) | sent; if ( use_direct_inject ) { - DIOCTL_DirectInjectReset msg = { p->daq_msg, !(flags & ENC_FLAG_FWD) }; + DIOCTL_DirectInjectReset msg = + { p->daq_msg, (uint8_t)((flags & ENC_FLAG_FWD) ? DAQ_DIR_FORWARD : DAQ_DIR_REVERSE) }; ret = p->daq_instance->ioctl(DIOCTL_DIRECT_INJECT_RESET, &msg, sizeof(msg)); if ( ret != DAQ_SUCCESS ) diff --git a/src/parser/parse_rule.cc b/src/parser/parse_rule.cc index c0ecf5341..3c261380c 100644 --- a/src/parser/parse_rule.cc +++ b/src/parser/parse_rule.cc @@ -1079,10 +1079,11 @@ void parse_rule_opt_set( if ( s_ignore ) return; + assert(val); if ( s_capture ) { s_body += opt; - if ( val and *val ) + if ( *val ) { s_body += " "; s_body += val; diff --git a/src/parser/parse_stream.cc b/src/parser/parse_stream.cc index f4bb96041..7328e3e43 100644 --- a/src/parser/parse_stream.cc +++ b/src/parser/parse_stream.cc @@ -479,7 +479,7 @@ struct RuleParseState string opt; string val; - bool tbd; + bool tbd = false; }; static bool exec( diff --git a/src/ports/port_table.cc b/src/ports/port_table.cc index 51dde27c4..bde982249 100644 --- a/src/ports/port_table.cc +++ b/src/ports/port_table.cc @@ -107,10 +107,10 @@ public: { unsigned char* pc_ptr = (unsigned char*)&plx->p[i]; - for ( unsigned k = 0; k < sizeof(void*); k++ ) + for ( unsigned j = 0; j < sizeof(void*); j++ ) { hash *= scale; - hash += pc_ptr[k]; + hash += pc_ptr[j]; } } return hash ^ hardener; @@ -722,6 +722,9 @@ int PortTableAddObject(PortTable* p, PortObject* po) { SF_LNODE* lpos; + if ( !p ) + return -1; + /* Search for the Port Object in the input list, by address */ for ( PortObject* pox = (PortObject*)sflist_first(p->pt_polist, &lpos); pox!=nullptr; @@ -853,13 +856,18 @@ void RuleListSortUniq(SF_LIST* rl) int* currNode = (int*)sflist_first(rl, &pos); if ( !currNode ) + { + snort_free(rlist); return; + } for ( unsigned i = 0; i < rl->count; i++ ) { if (rlist[i] > lastRuleIndex) { - *currNode = lastRuleIndex = rlist[i]; + lastRuleIndex = rlist[i]; + if (currNode) + *currNode = lastRuleIndex; //replace the next element in place currNode = (int*)sflist_next(&pos); uniqElements++; diff --git a/src/sfip/sf_ipvar.cc b/src/sfip/sf_ipvar.cc index 2aef07dc3..187bf62e2 100644 --- a/src/sfip/sf_ipvar.cc +++ b/src/sfip/sf_ipvar.cc @@ -817,10 +817,6 @@ SfIpRet sfvar_parse_iplist(vartable_t* table, sfip_var_t* var, { sfip_node_t* node; - /* Skip leading commas */ - for (; *str == ','; str++) - ; - /* Check for a negated "any" */ if (negation ^ neg_ip && !strcasecmp(tok, "any")) {