From: Russ Combs (rucombs) Date: Wed, 17 Oct 2018 21:41:01 +0000 (-0400) Subject: Merge pull request #1388 in SNORT/snort3 from fixits to master X-Git-Tag: 3.0.0-249~27 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9e8cb9f3cbb2347bf58d8e5e326d92d56c1c6d79;p=thirdparty%2Fsnort3.git Merge pull request #1388 in SNORT/snort3 from fixits to master Squashed commit of the following: commit a4d3e3e2137461904b12fd084f2479ced5054b39 Author: russ Date: Wed Oct 17 13:55:55 2018 -0400 comments: additional cleanup commit 760447cd21cbbad638554361a530637b7a09172a Author: Russ Combs Date: Mon Oct 15 10:13:19 2018 -0400 comments: remove XXX and convert to FIXIT where appropriate commit e950bb0de085322a28d0d830b321b821164113c2 Author: Russ Combs Date: Fri Oct 12 21:43:36 2018 -0400 fixits: prioritize for RC commit 071538dc340e457c64f151d28527b8b16e86fddf Author: Russ Combs Date: Fri Oct 12 21:38:06 2018 -0400 source: minor refactoring commit f9bcf0eb3098f2a9b324e5037b83c0915fa5c1ec Author: Russ Combs Date: Fri Oct 12 21:32:42 2018 -0400 comments: fixup format, spelling, priority, etc. commit ea1dcefeee1e3f280c85e5c38033bb0762c83ee5 Author: Russ Combs Date: Fri Oct 12 20:49:46 2018 -0400 build: remove dead code commit 7a77cb9c3e2e008be8450cae16be7b7ed777cb63 Author: Russ Combs Date: Fri Oct 12 20:38:26 2018 -0400 comments: delete obsolete comments commit b11dfc89c102cd73ef969ff18d86e4fe7c1df8cd Author: Russ Combs Date: Fri Oct 12 19:29:00 2018 -0400 build: support dynamic imap, pop, and smtp --- diff --git a/src/codecs/ip/cd_frag.cc b/src/codecs/ip/cd_frag.cc index 2383e6688..db9c79708 100644 --- a/src/codecs/ip/cd_frag.cc +++ b/src/codecs/ip/cd_frag.cc @@ -95,13 +95,13 @@ bool Ipv6FragCodec::decode(const RawData& raw, CodecData& codec, DecodeData& sno codec.proto_bits |= PROTO_BIT__IP6_EXT; codec.ip6_extension_count++; - // FIXIT-H the comment says to call it after setting next_prot_id, + // FIXIT-RC the comment says to call it after setting next_prot_id, // but it looks like it's called (twice) before setting it. // must be called AFTER setting next_prot_id CheckIPv6ExtensionOrder(codec, IpProtocol::FRAGMENT); - // FIXIT-H this breaks the tests/ips/normalize/ip6/would_opts_nop test + // FIXIT-RC this breaks the tests/ips/normalize/ip6/would_opts_nop test // because ip6frag_hdr->ip6f_nxt is set to FINISHED_DECODE here. (or // maybe the test has the wrong expected data). diff --git a/src/codecs/ip/cd_icmp4.cc b/src/codecs/ip/cd_icmp4.cc index 00fc184cd..188854b0d 100644 --- a/src/codecs/ip/cd_icmp4.cc +++ b/src/codecs/ip/cd_icmp4.cc @@ -432,13 +432,6 @@ void Icmp4Codec::log(TextLog* const log, const uint8_t* raw_pkt, break; } -/* written this way since inet_ntoa was typedef'ed to use sfip_ntoa - * which requires SfIp instead of inaddr's. This call to inet_ntoa - * is a rare case that doesn't use SfIp's. */ - -// XXX-IPv6 NOT YET IMPLEMENTED - IPV6 addresses technically not supported - need to change ICMP - - /* no inet_ntop in Windows */ snort_inet_ntop(AF_INET, (const void*)(&icmph->s_icmp_gwaddr.s_addr), buf, sizeof(buf)); TextLog_Print(log, " NEW GW: %s", buf); diff --git a/src/codecs/ip/cd_ipv4.cc b/src/codecs/ip/cd_ipv4.cc index a2e9404b0..a904ff57d 100644 --- a/src/codecs/ip/cd_ipv4.cc +++ b/src/codecs/ip/cd_ipv4.cc @@ -551,7 +551,7 @@ void Ipv4Codec::log(TextLog* const text_log, const uint8_t* raw_pkt, { const ip::IP4Hdr* const ip4h = reinterpret_cast(raw_pkt); - // FIXIT-H this does NOT obfuscate correctly + // FIXIT-RC this does NOT obfuscate correctly if (snort::SnortConfig::obfuscate()) { TextLog_Print(text_log, "xxx.xxx.xxx.xxx -> xxx.xxx.xxx.xxx"); diff --git a/src/codecs/ip/cd_ipv6.cc b/src/codecs/ip/cd_ipv6.cc index 95bd0b004..ac3cbc944 100644 --- a/src/codecs/ip/cd_ipv6.cc +++ b/src/codecs/ip/cd_ipv6.cc @@ -543,7 +543,7 @@ void Ipv6Codec::log(TextLog* const text_log, const uint8_t* raw_pkt, { const ip::IP6Hdr* const ip6h = reinterpret_cast(raw_pkt); - // FIXIT-H this does NOT obfuscate correctly + // FIXIT-RC this does NOT obfuscate correctly if (SnortConfig::obfuscate()) { TextLog_Print(text_log, "x:x:x:x::x:x:x:x -> x:x:x:x::x:x:x:x"); diff --git a/src/codecs/ip/cd_udp.cc b/src/codecs/ip/cd_udp.cc index 5da69d3a8..13988bd20 100644 --- a/src/codecs/ip/cd_udp.cc +++ b/src/codecs/ip/cd_udp.cc @@ -172,7 +172,7 @@ bool UdpCodec::decode(const RawData& raw, CodecData& codec, DecodeData& snort) const udp::UDPHdr* const udph = reinterpret_cast(raw.data); - // FIXIT-M since we no longer let UDP fragments through, erase extra code + // FIXIT-RC since we no longer let UDP fragments through, erase extra code if ((snort.decode_flags & DECODE_FRAG) == 0) { uhlen = ntohs(udph->uh_len); diff --git a/src/codecs/ip/checksum.h b/src/codecs/ip/checksum.h index b4095e2fe..9f0db58aa 100644 --- a/src/codecs/ip/checksum.h +++ b/src/codecs/ip/checksum.h @@ -147,7 +147,7 @@ inline uint16_t cksum_add(const uint16_t* buf, std::size_t len, uint32_t cksum) } sp += sn; - /* XXX - unroll loop using Duff's device. */ + /* unroll loop using Duff's device. */ while (--n > 0) { cksum += sp[0]; diff --git a/src/codecs/misc/cd_icmp4_ip.cc b/src/codecs/misc/cd_icmp4_ip.cc index aedf03a42..a99afaa76 100644 --- a/src/codecs/misc/cd_icmp4_ip.cc +++ b/src/codecs/misc/cd_icmp4_ip.cc @@ -150,7 +150,7 @@ void Icmp4IpCodec::log(TextLog* const text_log, const uint8_t* raw_pkt, // COPIED DIRECTLY FROM ipv4 CODEC. This is specifically replicated since // the two are not necessarily the same. - // FIXIT-H this does NOT obfuscate correctly + // FIXIT-RC this does NOT obfuscate correctly if (SnortConfig::obfuscate()) { TextLog_Print(text_log, "xxx.xxx.xxx.xxx -> xxx.xxx.xxx.xxx"); diff --git a/src/codecs/misc/cd_icmp6_ip.cc b/src/codecs/misc/cd_icmp6_ip.cc index e394d816a..d55e344d8 100644 --- a/src/codecs/misc/cd_icmp6_ip.cc +++ b/src/codecs/misc/cd_icmp6_ip.cc @@ -80,10 +80,10 @@ bool Icmp6IpCodec::decode(const RawData& raw, CodecData& codec, DecodeData&) return false; } -// orig_frag_offset = ntohs(GET_ORIG_IPH_OFF(p)); -// orig_frag_offset &= 0x1FFF; + // orig_frag_offset = ntohs(GET_ORIG_IPH_OFF(p)); + // orig_frag_offset &= 0x1FFF; - // XXX NOT YET IMPLEMENTED - fragments inside ICMP payload + // FIXIT-L NOT YET IMPLEMENTED - fragments inside ICMP payload // since we know the protocol ID in this layer (and NOT the // next layer), set the correct protocol here. Normally, diff --git a/src/connectors/file_connector/test/file_connector_test.cc b/src/connectors/file_connector/test/file_connector_test.cc index b4415f465..2a0572b6e 100644 --- a/src/connectors/file_connector/test/file_connector_test.cc +++ b/src/connectors/file_connector/test/file_connector_test.cc @@ -87,7 +87,7 @@ TEST_GROUP(file_connector) { void setup() override { - // FIXIT-L workaround for CppUTest mem leak detector issue + // FIXIT-RC workaround for CppUTest mem leak detector issue MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); fc_api = (ConnectorApi*)file_connector; connector_tx_text_config.direction = Connector::CONN_TRANSMIT; @@ -137,7 +137,7 @@ TEST_GROUP(file_connector_tinit_tterm) { void setup() override { - // FIXIT-L workaround for CppUTest mem leak detector issue + // FIXIT-RC workaround for CppUTest mem leak detector issue MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); fc_api = (ConnectorApi*)file_connector; connector_tx_text_config.direction = Connector::CONN_TRANSMIT; @@ -199,7 +199,7 @@ TEST_GROUP(file_connector_text) { void setup() override { - // FIXIT-L workaround for CppUTest mem leak detector issue + // FIXIT-RC workaround for CppUTest mem leak detector issue MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); fc_api = (ConnectorApi*)file_connector; connector_tx_text_config.direction = Connector::CONN_TRANSMIT; @@ -265,7 +265,7 @@ TEST_GROUP(file_connector_binary) { void setup() override { - // FIXIT-L workaround for CppUTest mem leak detector issue + // FIXIT-RC workaround for CppUTest mem leak detector issue MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); fc_api = (ConnectorApi*)file_connector; connector_tx_binary_config.direction = Connector::CONN_TRANSMIT; diff --git a/src/connectors/tcp_connector/test/tcp_connector_test.cc b/src/connectors/tcp_connector/test/tcp_connector_test.cc index 4691219a2..f4c4036ee 100644 --- a/src/connectors/tcp_connector/test/tcp_connector_test.cc +++ b/src/connectors/tcp_connector/test/tcp_connector_test.cc @@ -196,7 +196,7 @@ TEST_GROUP(tcp_connector) { void setup() override { - // FIXIT-L workaround for CppUTest mem leak detector issue + // FIXIT-RC workaround for CppUTest mem leak detector issue //MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); tcpc_api = (ConnectorApi*)tcp_connector; connector_config.direction = Connector::CONN_DUPLEX; @@ -236,7 +236,7 @@ TEST_GROUP(tcp_connector_call_error) { void setup() override { - // FIXIT-L workaround for CppUTest mem leak detector issue + // FIXIT-RC workaround for CppUTest mem leak detector issue //MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); tcpc_api = (ConnectorApi*)tcp_connector; set_normal_status(); @@ -267,7 +267,7 @@ TEST_GROUP(tcp_connector_call_other) { void setup() override { - // FIXIT-L workaround for CppUTest mem leak detector issue + // FIXIT-RC workaround for CppUTest mem leak detector issue //MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); } @@ -281,7 +281,7 @@ TEST_GROUP(tcp_connector_answer_error) { void setup() override { - // FIXIT-L workaround for CppUTest mem leak detector issue + // FIXIT-RC workaround for CppUTest mem leak detector issue //MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); tcpc_api = (ConnectorApi*)tcp_connector; set_normal_status(); @@ -369,7 +369,7 @@ TEST_GROUP(tcp_connector_tinit_tterm_thread_call) { void setup() override { - // FIXIT-L workaround for CppUTest mem leak detector issue + // FIXIT-RC workaround for CppUTest mem leak detector issue //MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); tcpc_api = (ConnectorApi*)tcp_connector; s_instance = 0; @@ -403,7 +403,7 @@ TEST_GROUP(tcp_connector_tinit_tterm_call) { void setup() override { - // FIXIT-L workaround for CppUTest mem leak detector issue + // FIXIT-RC workaround for CppUTest mem leak detector issue //MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); tcpc_api = (ConnectorApi*)tcp_connector; s_instance = 0; @@ -437,7 +437,7 @@ TEST_GROUP(tcp_connector_no_tinit_tterm_call) { void setup() override { - // FIXIT-L workaround for CppUTest mem leak detector issue + // FIXIT-RC workaround for CppUTest mem leak detector issue //MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); tcpc_api = (ConnectorApi*)tcp_connector; s_instance = 0; @@ -514,7 +514,7 @@ TEST_GROUP(tcp_connector_tinit_tterm_answer) { void setup() override { - // FIXIT-L workaround for CppUTest mem leak detector issue + // FIXIT-RC workaround for CppUTest mem leak detector issue //MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); s_instance = 0; set_normal_status(); diff --git a/src/decompress/file_decomp.h b/src/decompress/file_decomp.h index 0b9e01e9b..89b1ee31b 100644 --- a/src/decompress/file_decomp.h +++ b/src/decompress/file_decomp.h @@ -118,7 +118,7 @@ struct fd_session_t uint32_t Total_Out; // total number of bytes output so far // Configuration settings - // FIXIT-L Compr_Depth and Decompr_Depth only support OHI and eventually should be removed + // FIXIT-RC Compr_Depth and Decompr_Depth only support OHI and eventually should be removed uint32_t Compr_Depth; uint32_t Decompr_Depth; uint32_t Modes; // Bit mapped set of potential file/algo modes diff --git a/src/decompress/file_decomp_pdf.h b/src/decompress/file_decomp_pdf.h index 3f5c7cfaf..a1cb1335d 100644 --- a/src/decompress/file_decomp_pdf.h +++ b/src/decompress/file_decomp_pdf.h @@ -30,7 +30,7 @@ #define FILTER_SPEC_BUF_LEN (40) #define PARSE_STACK_LEN (12) -/* FIXIT-L Other than the API prototypes, the other parts of this header should +/* FIXIT-RC Other than the API prototypes, the other parts of this header should be private to file_decomp_pdf. */ enum fd_PDF_States @@ -79,7 +79,7 @@ struct fd_PDF_t uint8_t State; }; -// FIXIT-L don't obfuscate pointers +// FIXIT-RC don't obfuscate pointers typedef fd_PDF_Parse_Stack_t* fd_PDF_Parse_Stack_p_t; typedef fd_PDF_Parse_t* fd_PDF_Parse_p_t; typedef fd_PDF_t* fd_PDF_p_t; diff --git a/src/decompress/file_decomp_swf.h b/src/decompress/file_decomp_swf.h index c0fb8e8df..bd3ee1d50 100644 --- a/src/decompress/file_decomp_swf.h +++ b/src/decompress/file_decomp_swf.h @@ -29,7 +29,7 @@ #include "file_decomp.h" -/* FIXIT-L Other than the API prototypes, the other parts of this header should +/* FIXIT-RC Other than the API prototypes, the other parts of this header should be private to file_decomp_swf. */ /* Both ZLIB & LZMA files have an uncompressed eight byte header. The signature is diff --git a/src/detection/context_switcher.h b/src/detection/context_switcher.h index d6a656265..2ab04c738 100644 --- a/src/detection/context_switcher.h +++ b/src/detection/context_switcher.h @@ -78,7 +78,7 @@ public: unsigned hold_count() const; bool can_hold() const - { return idle_count() > 5; } // FIXIT-H define appropriate const + { return idle_count() > 5; } // FIXIT-RC define appropriate const bool on_hold(snort::Flow*); diff --git a/src/detection/detect.cc b/src/detection/detect.cc index 2f3e4900b..95434cef9 100644 --- a/src/detection/detect.cc +++ b/src/detection/detect.cc @@ -104,7 +104,7 @@ void CallAlertFuncs(Packet* p, const OptTreeNode* otn, ListHead* head) pc.total_alert_pkts++; #if 0 - // FIXIT-M this should be a generic feature of otn + // FIXIT-RC DELETE THIS this should be a generic feature of otn if ( otn->sigInfo.gid != GID_REPUTATION ) { /* Don't include IP Reputation events in count */ diff --git a/src/detection/detection_util.cc b/src/detection/detection_util.cc index a3af53e08..40474820e 100644 --- a/src/detection/detection_util.cc +++ b/src/detection/detection_util.cc @@ -96,7 +96,6 @@ void EventTrace_Log(const Packet* p, const OptTreeNode* otn, int action) "Pkt Cnts: Dsz=%u, Alt=%u\n", (unsigned)p->dsize, (unsigned)p->alt_dsize); - // FIXIT-L delete alt_dsize (only set by OHI) uint16_t n = p->alt_dsize > 0 ? p->alt_dsize : p->dsize; LogBuffer("Packet", p->data, n); diff --git a/src/detection/detection_util.h b/src/detection/detection_util.h index 268cce93c..d76483d5d 100644 --- a/src/detection/detection_util.h +++ b/src/detection/detection_util.h @@ -40,7 +40,7 @@ struct DataBuffer unsigned len; }; -// FIXIT-L event trace should be placed in its own files +// FIXIT-RC event trace should be placed in its own files void EventTrace_Init(); void EventTrace_Term(); diff --git a/src/detection/fp_create.cc b/src/detection/fp_create.cc index fcceb2014..aedfe8f05 100644 --- a/src/detection/fp_create.cc +++ b/src/detection/fp_create.cc @@ -811,9 +811,8 @@ static int fpGetFinalPattern( if (bytes < (int)pmd->pattern_size) { - /* The pattern is all '\0' - use the whole pattern - * XXX This potentially hurts the performance boost - * gained by stripping leading zeros */ + // The pattern is all '\0' - use the whole pattern. This potentially + // hurts the performance boost gained by stripping leading zeros. if (bytes == 0) { bytes = pmd->pattern_size; diff --git a/src/detection/fp_detect.cc b/src/detection/fp_detect.cc index 41209b87f..6b7a09eee 100644 --- a/src/detection/fp_detect.cc +++ b/src/detection/fp_detect.cc @@ -796,7 +796,7 @@ bool MpseStash::push(void* user, void* tree, int index, void* list) bool MpseStash::process(MpseMatch match, void* context) { if ( !enable ) - return true; // maxed out - quit, FIXIT-H count this condition + return true; // maxed out - quit, FIXIT-RC count this condition if ( count > pmqs.max_inq ) pmqs.max_inq = count; @@ -887,7 +887,7 @@ static inline int search_buffer( { if ( Mpse* so = omd->pg->mpse[pmt] ) { - // FIXIT-H get the context packet number + // FIXIT-H DELETE ME done - get the context packet number trace_logf(detection, TRACE_FP_SEARCH, "%" PRIu64 " fp %s.%s[%d]\n", omd->p->context->packet_number, gadget->get_name(), pm_type_strings[pmt], buf.len); diff --git a/src/detection/rtn_checks.cc b/src/detection/rtn_checks.cc index 888781ada..a6db45616 100644 --- a/src/detection/rtn_checks.cc +++ b/src/detection/rtn_checks.cc @@ -49,12 +49,10 @@ using namespace snort; static int CheckAddrPort(sfip_var_t* rule_addr, PortObject* po, Packet* p, uint32_t flags, int mode) { - const SfIp* pkt_addr; /* packet IP address */ - unsigned short pkt_port; /* packet port */ - int global_except_addr_flag = 0; /* global exception flag is set */ - int any_port_flag = 0; /* any port flag set */ - int except_port_flag = 0; /* port exception flag set */ - int ip_match = 0; /* flag to indicate addr match made */ + const SfIp* pkt_addr; + unsigned short pkt_port; + int any_port_flag = 0; + int ip_match = 0; /* set up the packet particulars */ if (mode & CHECK_SRC_IP) @@ -63,17 +61,9 @@ static int CheckAddrPort(sfip_var_t* rule_addr, PortObject* po, Packet* p, pkt_port = p->ptrs.sp; if (mode & INVERSE) - { - global_except_addr_flag = flags & EXCEPT_DST_IP; any_port_flag = flags & ANY_DST_PORT; - except_port_flag = flags & EXCEPT_DST_PORT; - } else - { - global_except_addr_flag = flags & EXCEPT_SRC_IP; any_port_flag = flags & ANY_SRC_PORT; - except_port_flag = flags & EXCEPT_SRC_PORT; - } } else { @@ -81,73 +71,32 @@ static int CheckAddrPort(sfip_var_t* rule_addr, PortObject* po, Packet* p, pkt_port = p->ptrs.dp; if (mode & INVERSE) - { - global_except_addr_flag = flags & EXCEPT_SRC_IP; any_port_flag = flags & ANY_SRC_PORT; - except_port_flag = flags & EXCEPT_SRC_PORT; - } else - { - global_except_addr_flag = flags & EXCEPT_DST_IP; any_port_flag = flags & ANY_DST_PORT; - except_port_flag = flags & EXCEPT_DST_PORT; - } } if (!rule_addr) goto bail; - if (!(global_except_addr_flag)) /*modeled after Check{Src,Dst}IP function*/ - { - if (sfvar_ip_in(rule_addr, pkt_addr)) - ip_match = 1; - } - else - { - /* global exception flag is up, we can't match on *any* - * of the source addresses - */ - - if (sfvar_ip_in(rule_addr, pkt_addr)) - return 0; - - ip_match=1; - } + if (sfvar_ip_in(rule_addr, pkt_addr)) + ip_match = 1; bail: if (!ip_match) - { return 0; - } /* if the any port flag is up, we're all done (success) */ if (any_port_flag) - { return 1; - } if (!(mode & (CHECK_SRC_PORT | CHECK_DST_PORT))) - { return 1; - } /* check the packet port against the rule port */ - if ( !PortObjectHasPort(po,pkt_port) ) - { - /* if the exception flag isn't up, fail */ - if (!except_port_flag) - { - return 0; - } - } - else - { - /* if the exception flag is up, fail */ - if (except_port_flag) - { - return 0; - } - } + /* if the exception flag isn't up, fail */ + if ( !PortObjectHasPort(po, pkt_port) ) + return 0; /* ports and address match */ return 1; @@ -168,8 +117,8 @@ int CheckBidirectional(Packet* p, RuleTreeNode* rtn_idx, if (CheckAddrPort(rtn_idx->dip, CHECK_ADDR_DST_ARGS(rtn_idx), p, rtn_idx->flags, (CHECK_SRC_IP | INVERSE | (check_ports ? CHECK_SRC_PORT : 0)))) { - if (!CheckAddrPort(rtn_idx->sip, CHECK_ADDR_SRC_ARGS(rtn_idx), p, - rtn_idx->flags, (CHECK_DST_IP | INVERSE | (check_ports ? CHECK_DST_PORT : 0)))) + if (!CheckAddrPort(rtn_idx->sip, CHECK_ADDR_SRC_ARGS(rtn_idx), p, rtn_idx->flags, + (CHECK_DST_IP | INVERSE | (check_ports ? CHECK_DST_PORT : 0)))) { return 0; } @@ -200,37 +149,13 @@ int CheckBidirectional(Packet* p, RuleTreeNode* rtn_idx, return 1; } -/**************************************************************************** - * - * Function: CheckSrcIp(Packet *, RuleTreeNode *, RuleFpList *) - * - * Purpose: Test the source IP and see if it equals the SIP of the packet - * - * Arguments: p => ptr to the decoded packet data structure - * rtn_idx => ptr to the current rule data struct - * fp_list => ptr to the current function pointer node - * - * Returns: 0 on failure (no match), 1 on success (match) - * - ***************************************************************************/ +// Purpose: Test the source IP and see if it equals the SIP of the packet +// Returns: 0 on failure (no match), 1 on success (match) int CheckSrcIP(Packet* p, RuleTreeNode* rtn_idx, RuleFpList* fp_list, int check_ports) { - if (!(rtn_idx->flags & EXCEPT_SRC_IP)) - { - if ( sfvar_ip_in(rtn_idx->sip, p->ptrs.ip_api.get_src()) ) - { - /* the packet matches this test, proceed to the next test */ - return fp_list->next->RuleHeadFunc(p, rtn_idx, fp_list->next, check_ports); - } - } - else + if ( sfvar_ip_in(rtn_idx->sip, p->ptrs.ip_api.get_src()) ) { - /* global exception flag is up, we can't match on *any* - * of the source addresses - */ - if ( sfvar_ip_in(rtn_idx->sip, p->ptrs.ip_api.get_src()) ) - return 0; - + /* the packet matches this test, proceed to the next test */ return fp_list->next->RuleHeadFunc(p, rtn_idx, fp_list->next, check_ports); } @@ -238,39 +163,15 @@ int CheckSrcIP(Packet* p, RuleTreeNode* rtn_idx, RuleFpList* fp_list, int check_ return 0; } -/**************************************************************************** - * - * Function: CheckDstIp(Packet *, RuleTreeNode *, RuleFpList *) - * - * Purpose: Test the dest IP and see if it equals the DIP of the packet - * - * Arguments: p => ptr to the decoded packet data structure - * rtn_idx => ptr to the current rule data struct - * fp_list => ptr to the current function pointer node - * - * Returns: 0 on failure (no match), 1 on success (match) - * - ***************************************************************************/ +// Purpose: Test the dest IP and see if it equals the DIP of the packet +// Returns: 0 on failure (no match), 1 on success (match) int CheckDstIP(Packet* p, RuleTreeNode* rtn_idx, RuleFpList* fp_list, int check_ports) { - if (!(rtn_idx->flags & EXCEPT_DST_IP)) - { - if ( sfvar_ip_in(rtn_idx->dip, p->ptrs.ip_api.get_dst()) ) - { - /* the packet matches this test, proceed to the next test */ - return fp_list->next->RuleHeadFunc(p, rtn_idx, fp_list->next, check_ports); - } - } - else + if ( sfvar_ip_in(rtn_idx->dip, p->ptrs.ip_api.get_dst()) ) { - /* global exception flag is up, we can't match on *any* - * of the source addresses */ - if ( sfvar_ip_in(rtn_idx->dip, p->ptrs.ip_api.get_dst()) ) - return 0; - + /* the packet matches this test, proceed to the next test */ return fp_list->next->RuleHeadFunc(p, rtn_idx, fp_list->next, check_ports); } - return 0; } diff --git a/src/detection/rules.h b/src/detection/rules.h index 07cc6bf99..751e025a5 100644 --- a/src/detection/rules.h +++ b/src/detection/rules.h @@ -26,16 +26,12 @@ #include "actions/actions.h" -#define EXCEPT_SRC_IP 0x0001 // FIXIT-L checked but not set, same as 2.X -#define EXCEPT_DST_IP 0x0002 // FIXIT-L checked but not set, same as 2.X -#define ANY_SRC_PORT 0x0004 -#define ANY_DST_PORT 0x0008 -#define ANY_FLAGS 0x0010 -#define EXCEPT_SRC_PORT 0x0020 -#define EXCEPT_DST_PORT 0x0040 -#define BIDIRECTIONAL 0x0080 -#define ANY_SRC_IP 0x0100 -#define ANY_DST_IP 0x0200 +#define ANY_SRC_PORT 0x01 +#define ANY_DST_PORT 0x02 +#define ANY_FLAGS 0x04 +#define BIDIRECTIONAL 0x08 +#define ANY_SRC_IP 0x10 +#define ANY_DST_IP 0x20 #define GID_DEFAULT 1 #define GID_SESSION 135 diff --git a/src/detection/signature.h b/src/detection/signature.h index 0bb7acfe5..8179e27d8 100644 --- a/src/detection/signature.h +++ b/src/detection/signature.h @@ -47,7 +47,6 @@ struct ReferenceSystemNode ReferenceSystemNode* ReferenceSystemAdd(snort::SnortConfig*, const char*, const char* = nullptr); -/* XXX: update to point to the ReferenceURLNode in the referenceURL list */ struct ReferenceNode { char* id; diff --git a/src/file_api/circular_buffer.h b/src/file_api/circular_buffer.h index 452da6d37..22dd2715b 100644 --- a/src/file_api/circular_buffer.h +++ b/src/file_api/circular_buffer.h @@ -28,7 +28,7 @@ #include "main/snort_types.h" -#define CB_SUCCESS 0 // FIXIT-L use bool +#define CB_SUCCESS 0 // FIXIT-RC use bool #define CB_FAIL (-1) // Opaque buffer element type. This would be defined by the application. @@ -42,8 +42,8 @@ CircularBuffer* cbuffer_init(uint64_t size); void cbuffer_free(CircularBuffer* cb); -int cbuffer_is_full(CircularBuffer* cb); // FIXIT-L use bool -int cbuffer_is_empty(CircularBuffer* cb); // FIXIT-L use bool +int cbuffer_is_full(CircularBuffer* cb); // FIXIT-RC use bool +int cbuffer_is_empty(CircularBuffer* cb); // FIXIT-RC use bool // Returns number of elements in use uint64_t cbuffer_used(CircularBuffer* cb); diff --git a/src/file_api/file_mempool.h b/src/file_api/file_mempool.h index 5555700bb..a76155328 100644 --- a/src/file_api/file_mempool.h +++ b/src/file_api/file_mempool.h @@ -34,7 +34,7 @@ #include "circular_buffer.h" -#define FILE_MEM_SUCCESS 0 // FIXIT-L use bool +#define FILE_MEM_SUCCESS 0 // FIXIT-RC use bool #define FILE_MEM_FAIL (-1) class FileMemPool diff --git a/src/file_api/file_module.h b/src/file_api/file_module.h index fca12fe42..ae860930d 100644 --- a/src/file_api/file_module.h +++ b/src/file_api/file_module.h @@ -54,11 +54,6 @@ public: void show_dynamic_stats() override; - // FIXIT-L delete file_id gid when bogus rules are eliminated - // (this ensures those rules don't fire on every packet) - unsigned get_gid() const override - { return 146; } - private: FileMagicRule rule; FileMagicData magic; diff --git a/src/file_api/file_stats.cc b/src/file_api/file_stats.cc index 325cb14b3..439a67bea 100644 --- a/src/file_api/file_stats.cc +++ b/src/file_api/file_stats.cc @@ -168,7 +168,7 @@ void file_stats_print() processed_total[0], processed_total[1]); #if 0 - LogLabel("file type verdicts"); // FIXIT-L what's up with this code + LogLabel("file type verdicts"); // FIXIT-RC should be fixed uint64_t verdicts_total = 0;#include "file_capture.h" for (unsigned i = 0; i < FILE_VERDICT_MAX; i++) diff --git a/src/file_api/file_stats.h b/src/file_api/file_stats.h index 993b916e3..4abbd1681 100644 --- a/src/file_api/file_stats.h +++ b/src/file_api/file_stats.h @@ -22,8 +22,6 @@ #ifndef FILE_STATS_H #define FILE_STATS_H -// FIXIT-M This will be refactored soon - #include "framework/counts.h" #include "main/thread.h" diff --git a/src/flow/flow_cache.cc b/src/flow/flow_cache.cc index 23f2af5de..99860506c 100644 --- a/src/flow/flow_cache.cc +++ b/src/flow/flow_cache.cc @@ -172,7 +172,7 @@ unsigned FlowCache::prune_stale(uint32_t thetime, const Flow* save_me) while ( flow and pruned <= cleanup_flows ) { #if 0 - // FIXIT-H this loops forever if 1 flow in cache + // FIXIT-RC this loops forever if 1 flow in cache if (flow == save_me) { break; diff --git a/src/flow/flow_control.cc b/src/flow/flow_control.cc index c07887d64..b661e6fe2 100644 --- a/src/flow/flow_control.cc +++ b/src/flow/flow_control.cc @@ -152,7 +152,7 @@ void FlowControl::timeout_flows(time_t cur_time) void FlowControl::preemptive_cleanup() { - // FIXIT-H is there a possibility of this looping forever? + // FIXIT-RC is there a possibility of this looping forever? while ( memory::MemoryCap::over_threshold() ) { if ( !prune_one(PruneReason::PREEMPTIVE, true) ) diff --git a/src/flow/flow_key.h b/src/flow/flow_key.h index 44633b7ec..4f9276ea2 100644 --- a/src/flow/flow_key.h +++ b/src/flow/flow_key.h @@ -69,7 +69,7 @@ struct SO_PUBLIC FlowKey void init_vlan(uint16_t); void init_address_space(uint16_t); - // XXX If this data structure changes size, compare must be updated! + // If this data structure changes size, compare must be updated! static uint32_t hash(HashFnc*, const unsigned char* d, int); static int compare(const void* s1, const void* s2, size_t); diff --git a/src/ips_options/ips_regex.cc b/src/ips_options/ips_regex.cc index 2066a63d5..1496671ba 100644 --- a/src/ips_options/ips_regex.cc +++ b/src/ips_options/ips_regex.cc @@ -113,7 +113,7 @@ RegexOption::RegexOption(const RegexConfig& c) : if ( /*hs_error_t err =*/ hs_alloc_scratch(config.db, &s_scratch) ) { - // FIXIT-L why is this failing but everything is working? + // FIXIT-RC why is this failing but everything is working? //ParseError("can't initialize regex for '%s' (%d) %p", // config.re.c_str(), err, s_scratch); } diff --git a/src/ips_options/ips_sd_pattern.cc b/src/ips_options/ips_sd_pattern.cc index 4f4346b2e..d16a3f2e5 100644 --- a/src/ips_options/ips_sd_pattern.cc +++ b/src/ips_options/ips_sd_pattern.cc @@ -53,9 +53,6 @@ using namespace snort; // and then clone to thread specific after all rules are loaded. s_scratch is // a prototype that is large enough for all uses. -// FIXIT-L Determine if it's worthwhile to use a single scratch space for both -// "regex" and "sd_pattern" keywords. -// FIXIT-L See ips_regex.cc for more information. static hs_scratch_t* s_scratch = nullptr; static unsigned scratch_index; diff --git a/src/latency/rule_latency.cc b/src/latency/rule_latency.cc index c70b2472f..20f6e46f0 100644 --- a/src/latency/rule_latency.cc +++ b/src/latency/rule_latency.cc @@ -159,7 +159,6 @@ struct DefaultRuleInterface for ( int i = 0; i < root.num_children; ++i ) { auto& child_state = root.children[i]->state[get_instance_id()]; - // FIXIT-L rename to something like latency_timeout_count ++child_state.latency_timeouts; ++child_state.latency_suspends; } @@ -171,7 +170,6 @@ struct DefaultRuleInterface { for ( int i = 0; i < root.num_children; ++i ) { - // FIXIT-L rename to something like latency_timeout_count ++root.children[i]->state[get_instance_id()].latency_timeouts; } } diff --git a/src/log/log_text.cc b/src/log/log_text.cc index 13a37ce2a..2c214f87e 100644 --- a/src/log/log_text.cc +++ b/src/log/log_text.cc @@ -660,7 +660,7 @@ static void LogEmbeddedICMPHeader(TextLog* log, const ICMPHdr* icmph) break; case ICMP_REDIRECT: -// XXX-IPv6 "NOT YET IMPLEMENTED - ICMP printing" + // FIXIT-L IPv6 not yet implemented - ICMP printing break; case ICMP_ECHO: @@ -917,13 +917,7 @@ void LogICMPHeader(TextLog* log, Packet* p) break; } -/* written this way since inet_ntoa was typedef'ed to use sfip_ntoa - * which requires SfIp instead of inaddr's. This call to inet_ntoa - * is a rare case that doesn't use SfIp's. */ - -// XXX-IPv6 NOT YET IMPLEMENTED - IPV6 addresses technically not supported - need to change ICMP - - /* no inet_ntop in Windows */ + // FIXIT-L IPv6 NOT YET IMPLEMENTED - need to change ICMP snort_inet_ntop(AF_INET, (const void*)(&p->ptrs.icmph->s_icmp_gwaddr.s_addr), buf, sizeof(buf)); TextLog_Print(log, " NEW GW: %s", buf); diff --git a/src/log/messages.h b/src/log/messages.h index 11c42bbdc..c2c7ede60 100644 --- a/src/log/messages.h +++ b/src/log/messages.h @@ -61,7 +61,7 @@ SO_PUBLIC void LogMessage(FILE* fh, const char*, ...) __attribute__((format (pri SO_PUBLIC void WarningMessage(const char*, ...) __attribute__((format (printf, 1, 2))); SO_PUBLIC void ErrorMessage(const char*, ...) __attribute__((format (printf, 1, 2))); -// FIXIT-M do not call FatalError() during runtime +// FIXIT-RC do not call FatalError() during runtime [[noreturn]] SO_PUBLIC void FatalError(const char*, ...) __attribute__((format (printf, 1, 2))); NORETURN_ASSERT void log_safec_error(const char*, void*, int); diff --git a/src/log/unified2.h b/src/log/unified2.h index cf2ac60e1..9eab58abf 100644 --- a/src/log/unified2.h +++ b/src/log/unified2.h @@ -33,6 +33,7 @@ // #define UNIFIED2_IDS_EVENT_IPV6 72 // #define UNIFIED2_IDS_EVENT_MPLS 99 // #define UNIFIED2_IDS_EVENT_IPV6_MPLS 100 +// #define UNIFIED2_IDS_EVENT_APPSTAT 113 // CURRENT #define UNIFIED2_PACKET 2 @@ -40,7 +41,6 @@ #define UNIFIED2_IDS_EVENT_VLAN 104 // legacy_events #define UNIFIED2_IDS_EVENT_IPV6_VLAN 105 // legacy_events #define UNIFIED2_EXTRA_DATA 110 -#define UNIFIED2_IDS_EVENT_APPSTAT 113 // FIXIT-L owned by appid (should have own # space) #define UNIFIED2_EVENT3 114 #define MAX_EVENT_APPNAME_LEN 64 diff --git a/src/loggers/alert_csv.cc b/src/loggers/alert_csv.cc index ac616cf48..9e9df913d 100644 --- a/src/loggers/alert_csv.cc +++ b/src/loggers/alert_csv.cc @@ -595,7 +595,7 @@ void CsvLogger::alert(Packet* p, const char* msg, const Event& event) if ( first ) first = false; else - // FIXIT-M need to check csv_log for nullptr + // FIXIT-RC need to check csv_log for nullptr TextLog_Puts(csv_log, sep.c_str()); f(a); diff --git a/src/lua/lua_util.h b/src/lua/lua_util.h index 1326cbdeb..04bd5f4ab 100644 --- a/src/lua/lua_util.h +++ b/src/lua/lua_util.h @@ -24,7 +24,7 @@ #include -#define LUA_DIR_SEP '/' // FIXIT-L do we really want to hardcode this? +#define LUA_DIR_SEP '/' #define SCRIPT_DIR_VARNAME "SCRIPT_DIR" namespace Lua diff --git a/src/main.cc b/src/main.cc index a9494df6f..2cf57b794 100644 --- a/src/main.cc +++ b/src/main.cc @@ -709,8 +709,6 @@ static bool set_mode() #ifdef PIGLET if ( Piglet::piglet_mode() ) { - // FIXIT-L the early return means that piglet and catch tests cannot - // be run in the same process main_exit_code = Piglet::main(); return false; } diff --git a/src/main/analyzer_command.cc b/src/main/analyzer_command.cc index 06ce4d30e..8695c1e7d 100644 --- a/src/main/analyzer_command.cc +++ b/src/main/analyzer_command.cc @@ -67,16 +67,18 @@ void ACRotate::execute(Analyzer&) void ACGetStats::execute(Analyzer&) { - // FIXIT-P This incurs locking on all threads to retrieve stats. It could be reimplemented to - // optimize for large thread counts by retrieving stats in the command and accumulating in the - // main thread. + + // FIXIT-P This incurs locking on all threads to retrieve stats. It + // could be reimplemented to optimize for large thread counts by + // retrieving stats in the command and accumulating in the main thread. ModuleManager::accumulate(snort::SnortConfig::get_conf()); } ACGetStats::~ACGetStats() { - // FIXIT-L This should track the owner so it can dump stats to the shell instead of the logs - // when initiated by a shell command + + // FIXIT-L This should track the owner so it can dump stats to the + // shell instead of the logs when initiated by a shell command DropStats(); } diff --git a/src/main/modules.cc b/src/main/modules.cc index 38319a13f..aa3fc339c 100755 --- a/src/main/modules.cc +++ b/src/main/modules.cc @@ -426,7 +426,7 @@ static const Parameter profiler_rule_params[] = { nullptr, Parameter::PT_MAX, nullptr, nullptr, nullptr } }; -static const Parameter profiler_params[] = // FIXIT-L add help +static const Parameter profiler_params[] = { { "modules", Parameter::PT_TABLE, profiler_time_params, nullptr, "module time profiling" }, diff --git a/src/main/modules.h b/src/main/modules.h index c3b4b5f15..ede696db9 100644 --- a/src/main/modules.h +++ b/src/main/modules.h @@ -22,7 +22,7 @@ #define MODULES_H // this is for builtin module initialization. -// ideally, modules.cc would be refactored and several files. +// ideally, modules.cc would be refactored into several files. #include "framework/counts.h" #include "main/snort_debug.h" diff --git a/src/main/snort.cc b/src/main/snort.cc index f91ac2ec6..d064e7f65 100644 --- a/src/main/snort.cc +++ b/src/main/snort.cc @@ -188,7 +188,7 @@ static void register_profiles() static void pass_pkts(Packet*) { } static MainHook_f main_hook = pass_pkts; -static void set_policy(Packet* p) // FIXIT-M delete this? +static void set_policy(Packet* p) { set_default_policy(); p->user_inspection_policy_id = get_inspection_policy()->user_policy_id; diff --git a/src/main/snort_config.cc b/src/main/snort_config.cc index 303a69de0..a040a5804 100644 --- a/src/main/snort_config.cc +++ b/src/main/snort_config.cc @@ -253,7 +253,7 @@ SnortConfig::~SnortConfig() if ( scratch_handlers[i - 1].second ) scratch_handlers[i - 1].second(this); } - // FIXIT-T: Do we need to shrink_to_fit() state->scratch at this point? + // FIXIT-L: Do we need to shrink_to_fit() state->scratch at this point? } FreeRuleLists(this); diff --git a/src/main/snort_config.h b/src/main/snort_config.h index 83acda4f4..ffe361a98 100644 --- a/src/main/snort_config.h +++ b/src/main/snort_config.h @@ -274,7 +274,7 @@ public: FastPatternConfig* fast_pattern_config = nullptr; EventQueueConfig* event_queue_config = nullptr; - /* XXX XXX policy specific? */ + /* policy specific? */ ThresholdConfig* threshold_config = nullptr; RateFilterConfig* rate_filter_config = nullptr; DetectionFilterConfig* detection_filter_config = nullptr; diff --git a/src/managers/action_manager.cc b/src/managers/action_manager.cc index 6177cc34e..162511104 100644 --- a/src/managers/action_manager.cc +++ b/src/managers/action_manager.cc @@ -87,7 +87,7 @@ static void store(const ActionApi* api, IpsAction* act) for ( auto& p : s_actors ) if ( p.api == api ) { - //assert(!p.act); FIXIT-H memory leak on reload; move to SnortConfig? + //assert(!p.act); FIXIT-RC memory leak on reload; move to SnortConfig? p.act = act; break; } diff --git a/src/managers/inspector_manager.cc b/src/managers/inspector_manager.cc index 386e426e1..3051cb206 100644 --- a/src/managers/inspector_manager.cc +++ b/src/managers/inspector_manager.cc @@ -695,8 +695,10 @@ void InspectorManager::thread_stop(SnortConfig*) set_default_policy(); InspectionPolicy* pi = snort::get_inspection_policy(); - // FIXIT-H Any inspectors that were once configured/instantiated but no longer exist in the conf - // cannot have their instance tterm() called and will leak! + // FIXIT-RC Any inspectors that were once configured/instantiated but + // no longer exist in the conf cannot have their instance tterm() + // called and will leak! + if ( pi && pi->framework_policy ) { for ( auto* p : pi->framework_policy->ilist ) diff --git a/src/managers/mpse_manager.cc b/src/managers/mpse_manager.cc index 748ed6527..c2c1d16a7 100644 --- a/src/managers/mpse_manager.cc +++ b/src/managers/mpse_manager.cc @@ -183,14 +183,6 @@ void MpseManager::print_qinfo() sfksearch_print_qinfo(); acsmx2_print_qinfo(); } - -// this is commented out of snort.cc -// combine with above? -void MpseManager::print_search_engine_stats() -{ - IntelPmPrintBufferStats(); -} - #endif #ifdef PIGLET diff --git a/src/managers/script_manager.cc b/src/managers/script_manager.cc index ae7f8aaa3..650ae947f 100644 --- a/src/managers/script_manager.cc +++ b/src/managers/script_manager.cc @@ -115,7 +115,7 @@ LogLuaApi::LogLuaApi(string& s, string& c, unsigned v) : LuaApi(s, c) // lua foo //------------------------------------------------------------------------- -// FIXIT-L could be a template +// could be a template static bool get_field(lua_State* L, const char* key, int& value) { lua_pushstring(L, key); diff --git a/src/mime/file_mime_decode.h b/src/mime/file_mime_decode.h index 6aece520e..87b484e5f 100644 --- a/src/mime/file_mime_decode.h +++ b/src/mime/file_mime_decode.h @@ -23,9 +23,13 @@ // Email attachment decoder, supports Base64, QP, UU, and Bit7/8 #include "framework/counts.h" +#include "main/snort_types.h" #include "mime/decode_base.h" #include "mime/file_mime_config.h" +namespace snort +{ + enum DecodeType { DECODE_NONE = 0, @@ -48,7 +52,7 @@ struct MimeStats PegCount bitenc_bytes; }; -class MimeDecode +class SO_PUBLIC MimeDecode { public: MimeDecode(snort::DecodeConfig* conf); @@ -77,5 +81,7 @@ private: DataDecode* decoder = nullptr; }; +} // namespace snort + #endif diff --git a/src/mime/file_mime_log.h b/src/mime/file_mime_log.h index cd31e5e5d..f375c69a3 100644 --- a/src/mime/file_mime_log.h +++ b/src/mime/file_mime_log.h @@ -25,6 +25,7 @@ // Email headers and emails are also stored in the log buffer #include +#include "main/snort_types.h" enum EmailUserType { @@ -46,7 +47,7 @@ namespace snort class Flow; } -class MailLogState +class SO_PUBLIC MailLogState { public: MailLogState(MailLogConfig* conf); diff --git a/src/mime/file_mime_process.cc b/src/mime/file_mime_process.cc index a21f8f74a..a454800e9 100644 --- a/src/mime/file_mime_process.cc +++ b/src/mime/file_mime_process.cc @@ -514,7 +514,7 @@ const uint8_t* MimeSession::process_mime_data_paf( if (data_state == STATE_DATA_INIT) data_state = STATE_DATA_HEADER; - /* XXX A line starting with a '.' that isn't followed by a '.' is + /* A line starting with a '.' that isn't followed by a '.' is * deleted (RFC 821 - 4.5.2. TRANSPARENCY). If data starts with * '. text', i.e a dot followed by white space then text, some * servers consider it data header and some data body. @@ -540,8 +540,9 @@ const uint8_t* MimeSession::process_mime_data_paf( if (normalize_data(start, end) < 0) return nullptr; - /* now we shouldn't have to worry about copying any data to the alt buffer - * * only mime headers if we find them and only if we're ignoring data */ + + // now we shouldn't have to worry about copying any data to the alt buffer + // only mime headers if we find them and only if we're ignoring data while ((start != nullptr) && (start < end)) { @@ -708,7 +709,6 @@ void MimeSession::init() mime_hdr_search_mpse->prep(); } -// Free anything that needs it before shutting down preprocessor void MimeSession::exit() { if (mime_hdr_search_mpse != nullptr) diff --git a/src/network_inspectors/appid/app_info_table.cc b/src/network_inspectors/appid/app_info_table.cc index 9e45cf482..b41b0f326 100644 --- a/src/network_inspectors/appid/app_info_table.cc +++ b/src/network_inspectors/appid/app_info_table.cc @@ -32,7 +32,6 @@ #include "appid_config.h" #include "appid_inspector.h" #include "appid_peg_counts.h" -#include "log/messages.h" #include "log/unified2.h" #include "main/snort_config.h" #include "target_based/snort_protocols.h" @@ -556,7 +555,7 @@ void AppInfoManager::init_appid_info_table(AppIdModuleConfig* mod_config, /* snort service key, if it exists */ token = strtok_r(nullptr, CONF_SEPARATORS, &context); - // FIXIT-H: Sometimes the token is "~". Should we ignore those? + // FIXIT-RC: Sometimes the token is "~". Should we ignore those? if (token) entry->snort_protocol_id = add_appid_protocol_reference(token, sc); diff --git a/src/network_inspectors/appid/appid_config.cc b/src/network_inspectors/appid/appid_config.cc index ede083f09..aa2318ae6 100644 --- a/src/network_inspectors/appid/appid_config.cc +++ b/src/network_inspectors/appid/appid_config.cc @@ -98,8 +98,7 @@ AppIdModuleConfig::~AppIdModuleConfig() snort_free((void*)app_detector_dir); } -//FIXIT-M: RELOAD - move initialization back to AppIdConfig -//class constructor +// FIXIT-M: RELOAD - move initialization back to AppIdConfig class constructor AppInfoManager& AppIdConfig::app_info_mgr = AppInfoManager::get_instance(); AppIdConfig::AppIdConfig(AppIdModuleConfig* config) @@ -133,8 +132,7 @@ AppIdConfig::~AppIdConfig() cleanup(); } -//FIXIT-M: RELOAD - Move app info table cleanup back -//to AppId config destructor - cleanup() +// FIXIT-M: RELOAD - Move app info table cleanup back to AppId config destructor - cleanup() void AppIdConfig::pterm() { AppIdConfig::app_info_mgr.cleanup_appid_info_table(); @@ -752,8 +750,8 @@ void AppIdConfig::set_safe_search_enforcement(bool enabled) bool AppIdConfig::init_appid(SnortConfig* sc, AppIdInspector *ins) { - //FIXIT -M: RELOAD - Get rid of "once" flag - //Handle the if condition in AppIdConfig::init_appid + // FIXIT-M: RELOAD - Get rid of "once" flag + // Handle the if condition in AppIdConfig::init_appid static bool once = false; if (!once) { diff --git a/src/network_inspectors/appid/appid_config.h b/src/network_inspectors/appid/appid_config.h index e5fa936ff..c959f92a5 100644 --- a/src/network_inspectors/appid/appid_config.h +++ b/src/network_inspectors/appid/appid_config.h @@ -142,8 +142,8 @@ private: void process_config_directive(char* toklist[], int /* reload */); int load_analysis_config(const char* config_file, int reload, int instance_id); void display_port_config(); - //FIXIT-M: RELOAD - Remove static, once app_info_mgr cleanup is - //removed from AppIdConfig::pterm + // FIXIT-M: RELOAD - Remove static, once app_info_mgr cleanup is + // removed from AppIdConfig::pterm static AppInfoManager& app_info_mgr; }; diff --git a/src/network_inspectors/appid/appid_module.cc b/src/network_inspectors/appid/appid_module.cc index 756ef073a..cae51f7df 100644 --- a/src/network_inspectors/appid/appid_module.cc +++ b/src/network_inspectors/appid/appid_module.cc @@ -180,15 +180,6 @@ static const Command appid_cmds[] = { nullptr, nullptr, nullptr, nullptr } }; -// FIXIT-M Add appid_rules back in once we start using it. -#ifdef REMOVED_WHILE_NOT_IN_USE -static const RuleMap appid_rules[] = -{ - { 0 /* rule id */, "description" }, - { 0, nullptr } -}; -#endif - static const PegInfo appid_pegs[] = { { CountType::SUM, "packets", "count of packets received" }, diff --git a/src/network_inspectors/appid/appid_session.cc b/src/network_inspectors/appid/appid_session.cc index 061765dbd..72ae4bb73 100644 --- a/src/network_inspectors/appid/appid_session.cc +++ b/src/network_inspectors/appid/appid_session.cc @@ -148,7 +148,7 @@ AppIdSession::~AppIdSession() snort_free(firewall_early_data); } -// FIXIT-L X Move this to somewhere more generally available/appropriate. +// FIXIT-RC X Move this to somewhere more generally available/appropriate (decode_data.h). static inline PktType get_pkt_type_from_ip_proto(IpProtocol proto) { switch (proto) @@ -177,8 +177,8 @@ AppIdSession* AppIdSession::create_future_session(const Packet* ctrlPkt, const S assert(type != PktType::NONE); - // FIXIT-M - port parameter passed in as 0 since we may not know client port, verify this is - // correct + // FIXIT-RC - port parameter passed in as 0 since we may not know client port, verify + AppIdSession* asd = new AppIdSession(proto, cliIp, 0, inspector); asd->common.policyId = asd->config->appIdPolicyId; diff --git a/src/network_inspectors/appid/appid_session.h b/src/network_inspectors/appid/appid_session.h index 1b677e4ba..07ebf2517 100644 --- a/src/network_inspectors/appid/appid_session.h +++ b/src/network_inspectors/appid/appid_session.h @@ -233,7 +233,7 @@ public: AppId misc_app_id = APP_ID_NONE; - // FIXIT-M netbios_name is never set to a valid value + // FIXIT-RC netbios_name is never set to a valid value; set when netbios_domain is set? char* netbios_name = nullptr; char* netbios_domain = nullptr; diff --git a/src/network_inspectors/appid/client_plugins/client_app_ssh.cc b/src/network_inspectors/appid/client_plugins/client_app_ssh.cc index 608fd449f..ba94c159b 100644 --- a/src/network_inspectors/appid/client_plugins/client_app_ssh.cc +++ b/src/network_inspectors/appid/client_plugins/client_app_ssh.cc @@ -289,11 +289,13 @@ static inline int ssh_client_validate_keyx(uint16_t offset, const uint8_t* data, if (fd->pos >= fd->plen) { offset++; - // FIXIT-L if offset > size then there is probably a D-H Key Exchange Init packet - // in this payload - // For now parsing the Key Exchange Init is good enough to declare valid key - // exchange but for - // future enhance parsing to validate the D-H Key Exchange Init. + + // FIXIT-L if offset > size then there is probably a D-H + // Key Exchange Init packet in this payload. For now parsing + // the Key Exchange Init is good enough to declare valid + // key exchange but for future enhance parsing to validate + // the D-H Key Exchange Init. + if (offset == size) return APPID_SUCCESS; else diff --git a/src/network_inspectors/appid/detector_plugins/detector_sip.cc b/src/network_inspectors/appid/detector_plugins/detector_sip.cc index 1b870d11f..7d4709816 100644 --- a/src/network_inspectors/appid/detector_plugins/detector_sip.cc +++ b/src/network_inspectors/appid/detector_plugins/detector_sip.cc @@ -330,31 +330,45 @@ static int get_sip_client_app(void* patternMatcher, const char* pattern, uint32_ void SipServiceDetector::createRtpFlow(AppIdSession& asd, const Packet* pkt, const SfIp* cliIp, uint16_t cliPort, const SfIp* srvIp, uint16_t srvPort, IpProtocol proto, int16_t app_id) { - // FIXIT-H: Passing app_id instead of SnortProtocolId to create_future_session is incorrect. We need to look up snort_protocol_id. - AppIdSession* fp = AppIdSession::create_future_session(pkt, cliIp, cliPort, srvIp, srvPort, - proto, app_id, APPID_EARLY_SESSION_FLAG_FW_RULE, handler->get_inspector()); + // FIXIT-RC: Passing app_id instead of SnortProtocolId to + // create_future_session is incorrect. We need to look up + // snort_protocol_id. + + AppIdSession* fp = AppIdSession::create_future_session( + pkt, cliIp, cliPort, srvIp, srvPort, proto, app_id, + APPID_EARLY_SESSION_FLAG_FW_RULE, handler->get_inspector()); + if ( fp ) { fp->client.set_id(asd.client.get_id()); fp->payload.set_id(asd.payload.get_id()); fp->service.set_id(APP_ID_RTP); + // FIXIT-H : snort 2.9.x updated the flag to APPID_SESSION_EXPECTED_EVALUATE. // Check if it is needed here as well. //initialize_expected_session(asd, fp, APPID_SESSION_EXPECTED_EVALUATE); - initialize_expected_session(asd, *fp, APPID_SESSION_IGNORE_ID_FLAGS, APP_ID_APPID_SESSION_DIRECTION_MAX); + + initialize_expected_session( + asd, *fp, APPID_SESSION_IGNORE_ID_FLAGS, APP_ID_APPID_SESSION_DIRECTION_MAX); } // create an RTCP flow as well - AppIdSession* fp2 = AppIdSession::create_future_session(pkt, cliIp, cliPort + 1, srvIp, - srvPort + 1, proto, app_id, APPID_EARLY_SESSION_FLAG_FW_RULE, handler->get_inspector()); + + AppIdSession* fp2 = AppIdSession::create_future_session( + pkt, cliIp, cliPort + 1, srvIp, srvPort + 1, proto, app_id, + APPID_EARLY_SESSION_FLAG_FW_RULE, handler->get_inspector()); + if ( fp2 ) { fp2->client.set_id(asd.client.get_id()); fp2->payload.set_id(asd.payload.get_id()); fp2->service.set_id(APP_ID_RTCP); + // FIXIT-H : same comment as above //initialize_expected_session(asd, fp2, APPID_SESSION_EXPECTED_EVALUATE); - initialize_expected_session(asd, *fp2, APPID_SESSION_IGNORE_ID_FLAGS, APP_ID_APPID_SESSION_DIRECTION_MAX); + + initialize_expected_session( + asd, *fp2, APPID_SESSION_IGNORE_ID_FLAGS, APP_ID_APPID_SESSION_DIRECTION_MAX); } } @@ -419,8 +433,11 @@ SipServiceDetector::SipServiceDetector(ServiceDiscovery* sd) { SIP_PORT, IpProtocol::TCP, false } }; - // FIXIT - detector instance in each packet thread is calling this single sip event handler, - // last guy end wins, works now because it is all the same but this is not right... + // FIXIT-RC - detector instance in each packet thread is calling this + // single sip event handler, last guy end wins, works now because it is + // all the same but this is not right... + // Does this still apply? + handler->get_inspector().get_sip_event_handler().set_service(this); handler->register_detector(name, this, proto); } diff --git a/src/network_inspectors/appid/detector_plugins/detector_smtp.cc b/src/network_inspectors/appid/detector_plugins/detector_smtp.cc index bbffdff4f..8748eb009 100644 --- a/src/network_inspectors/appid/detector_plugins/detector_smtp.cc +++ b/src/network_inspectors/appid/detector_plugins/detector_smtp.cc @@ -181,9 +181,10 @@ SmtpClientDetector::SmtpClientDetector(ClientDiscovery* cdm) * including the NUL terminating character. */ // FIXIT-M - refactor this to reduce the number of function parameters -int SmtpClientDetector::extract_version_and_add_client_app(AppId clientId, const int prefix_len, - const uint8_t* product, const uint8_t* product_end, ClientSMTPData* const client_data, - AppIdSession& asd, AppId appId, AppidChangeBits& change_bits) +int SmtpClientDetector::extract_version_and_add_client_app( + AppId clientId, const int prefix_len, const uint8_t* product, const uint8_t* product_end, + ClientSMTPData* const client_data, AppIdSession& asd, AppId appId, + AppidChangeBits& change_bits) { uint8_t* v_end = client_data->version + MAX_VERSION_SIZE - 1; @@ -831,11 +832,13 @@ int SmtpServiceDetector::validate(AppIdDiscoveryArgs& args) if (fd->code == 220) { dd->client.flags |= CLIENT_FLAG_STARTTLS_SUCCESS; - //FIXIT-M: FIXIT-M: Revisit SSL decryption countdown after isSSLPolicyEnabled() is ported. - //Can we use Flow::is_proxied() here? + + // FIXIT-M: Revisit SSL decryption countdown after isSSLPolicyEnabled() + // is ported. Can we use Flow::is_proxied() here? #if 0 if (_dpd.isSSLPolicyEnabled(NULL)) #endif + dd->client.decryption_countdown = SSL_WAIT_PACKETS; // start a countdown #if 0 else diff --git a/src/network_inspectors/appid/detector_plugins/http_url_patterns.cc b/src/network_inspectors/appid/detector_plugins/http_url_patterns.cc index be33f3735..fd9e1db93 100644 --- a/src/network_inspectors/appid/detector_plugins/http_url_patterns.cc +++ b/src/network_inspectors/appid/detector_plugins/http_url_patterns.cc @@ -788,7 +788,7 @@ static int http_field_pattern_match(void* id, void*, int match_end_pos, void* da return 1; } -// FIXIT-M: Is this still necessary now that we use inspection events? +// FIXIT-RC: Is this still necessary now that we use inspection events? void HttpPatternMatchers::get_http_offsets(snort::Packet* pkt, AppIdHttpSession* hsession) { constexpr auto MIN_HTTP_REQ_HEADER_SIZE = (sizeof("GET /\r\n\r\n") - 1); diff --git a/src/network_inspectors/appid/lua_detector_api.cc b/src/network_inspectors/appid/lua_detector_api.cc index 23eda87a4..488f40407 100644 --- a/src/network_inspectors/appid/lua_detector_api.cc +++ b/src/network_inspectors/appid/lua_detector_api.cc @@ -2114,7 +2114,7 @@ static int add_port_pattern_client(lua_State* L) int index = 1; IpProtocol protocol = (IpProtocol)lua_tonumber(L, ++index); - uint16_t port = 0; //port = lua_tonumber(L, ++index); FIXIT-L - why commented out? + uint16_t port = 0; // port = lua_tonumber(L, ++index); FIXIT-RC - why commented out? const char* pattern = lua_tolstring(L, ++index, &patternSize); unsigned position = lua_tonumber(L, ++index); AppId appId = lua_tointeger(L, ++index); diff --git a/src/network_inspectors/appid/lua_detector_api.h b/src/network_inspectors/appid/lua_detector_api.h index f93865832..3f126b5df 100644 --- a/src/network_inspectors/appid/lua_detector_api.h +++ b/src/network_inspectors/appid/lua_detector_api.h @@ -104,8 +104,7 @@ public: }; -//FIXIT-M: RELOAD - Don't use this class, -//required now to store LSD objects +// FIXIT-M: RELOAD - Don't use this class, required now to store LSD objects class LuaObject { public: diff --git a/src/network_inspectors/appid/lua_detector_module.cc b/src/network_inspectors/appid/lua_detector_module.cc index a816fdee4..5cee62ffe 100644 --- a/src/network_inspectors/appid/lua_detector_module.cc +++ b/src/network_inspectors/appid/lua_detector_module.cc @@ -174,8 +174,8 @@ LuaDetectorManager::~LuaDetectorManager() lua_getfield(L, -1, lsd->package_info.cleanFunctionName.c_str()); if ( lua_isfunction(L, -1) ) { - //FIXIT-M: RELOAD - use lua references to get user data object from stack - //first parameter is DetectorUserData + // FIXIT-M: RELOAD - use lua references to get user data object from stack + // first parameter is DetectorUserData std::string name = lsd->package_info.name + "_"; lua_getglobal(L, name.c_str()); @@ -201,8 +201,10 @@ void LuaDetectorManager::initialize(AppIdConfig& config, int is_control) return; lua_detector_mgr = new LuaDetectorManager(config, is_control); + if (!lua_detector_mgr->L) - FatalError("Error - appid: can not create new luaState, instance=%u\n", get_instance_id()); + FatalError("Error - appid: can not create new luaState, instance=%u\n", + get_instance_id()); lua_detector_mgr->initialize_lua_detectors(); lua_detector_mgr->activate_lua_detectors(); diff --git a/src/network_inspectors/appid/service_plugins/service_detector.h b/src/network_inspectors/appid/service_plugins/service_detector.h index 6b0cd1462..4b5076c20 100644 --- a/src/network_inspectors/appid/service_plugins/service_detector.h +++ b/src/network_inspectors/appid/service_plugins/service_detector.h @@ -31,16 +31,21 @@ class ServiceDetector : public AppIdDetector { public: ServiceDetector(); + void do_custom_init() override { } void release_thread_resources() override { } void register_appid(AppId, unsigned extractsInfo) override; + int service_inprocess(AppIdSession&, const snort::Packet*, AppidSessionDirection dir); - int add_service(AppidChangeBits&, AppIdSession&, const snort::Packet*, AppidSessionDirection dir, AppId, - const char* vendor = nullptr, const char* version = nullptr, - const snort::AppIdServiceSubtype* = nullptr); + + int add_service(AppidChangeBits&, AppIdSession&, const snort::Packet*, + AppidSessionDirection, AppId, const char* vendor = nullptr, + const char* version = nullptr, const snort::AppIdServiceSubtype* = nullptr); + int add_service_consume_subtype(AppIdSession&, const snort::Packet*, AppidSessionDirection dir, AppId, const char* vendor, const char* version, snort::AppIdServiceSubtype*, AppidChangeBits&); + int incompatible_data(AppIdSession&, const snort::Packet*, AppidSessionDirection dir); int fail_service(AppIdSession&, const snort::Packet*, AppidSessionDirection dir); diff --git a/src/network_inspectors/appid/service_plugins/service_ftp.cc b/src/network_inspectors/appid/service_plugins/service_ftp.cc index e48c01caf..17771b7aa 100644 --- a/src/network_inspectors/appid/service_plugins/service_ftp.cc +++ b/src/network_inspectors/appid/service_plugins/service_ftp.cc @@ -798,7 +798,7 @@ void FtpServiceDetector::create_expected_session(AppIdSession& asd, const Packet uint16_t cliPort, const SfIp* srvIp, uint16_t srvPort, IpProtocol proto, int flags, AppidSessionDirection dir) { - //FIXIT-M - Avoid thread locals + // FIXIT-M - Avoid thread locals static THREAD_LOCAL SnortProtocolId ftp_data_snort_protocol_id = UNKNOWN_PROTOCOL_ID; if(ftp_data_snort_protocol_id == UNKNOWN_PROTOCOL_ID) ftp_data_snort_protocol_id = SnortConfig::get_conf()->proto_ref->find("ftp-data"); diff --git a/src/network_inspectors/appid/service_plugins/service_rexec.cc b/src/network_inspectors/appid/service_plugins/service_rexec.cc index 69264fa0e..04e6b1012 100644 --- a/src/network_inspectors/appid/service_plugins/service_rexec.cc +++ b/src/network_inspectors/appid/service_plugins/service_rexec.cc @@ -103,7 +103,7 @@ int RexecServiceDetector::validate(AppIdDiscoveryArgs& args) uint32_t port = 0; const uint8_t* data = args.data; uint16_t size = args.size; - //FIXIT-M - Avoid thread locals + // FIXIT-M - Avoid thread locals static THREAD_LOCAL SnortProtocolId rexec_snort_protocol_id = UNKNOWN_PROTOCOL_ID; ServiceREXECData* rd = (ServiceREXECData*)data_get(args.asd); diff --git a/src/network_inspectors/appid/service_plugins/service_rpc.cc b/src/network_inspectors/appid/service_plugins/service_rpc.cc index 6fad6cb75..b4551f3f8 100644 --- a/src/network_inspectors/appid/service_plugins/service_rpc.cc +++ b/src/network_inspectors/appid/service_plugins/service_rpc.cc @@ -190,7 +190,7 @@ RpcServiceDetector::RpcServiceDetector(ServiceDiscovery* sd) { if (rpc->r_name) { - // FIXIT-M - the memory allocate here may not be freed... + // FIXIT-RC - the memory allocated here may not be freed... prog = (RPCProgram*)snort_calloc(sizeof(RPCProgram)); prog->program = rpc->r_number; prog->next = rpc_programs; @@ -283,7 +283,7 @@ int RpcServiceDetector::validate_packet(const uint8_t* data, uint16_t size, Appi uint32_t val = 0; const uint8_t* end = nullptr; const RPCProgram* rprog = nullptr; - //FIXIT-M - Avoid thread locals + // FIXIT-M - Avoid thread locals static THREAD_LOCAL SnortProtocolId sunrpc_snort_protocol_id = UNKNOWN_PROTOCOL_ID; if (!size) diff --git a/src/network_inspectors/appid/service_plugins/service_rshell.cc b/src/network_inspectors/appid/service_plugins/service_rshell.cc index 2f5994050..9088b91eb 100644 --- a/src/network_inspectors/appid/service_plugins/service_rshell.cc +++ b/src/network_inspectors/appid/service_plugins/service_rshell.cc @@ -272,7 +272,8 @@ int RshellServiceDetector::validate(AppIdDiscoveryArgs& args) case RSHELL_STATE_STDERR_CONNECT_SYN_ACK: if (rd->parent && rd->parent->state == RSHELL_STATE_SERVER_CONNECT) rd->parent->state = RSHELL_STATE_USERNAME; - args.asd.set_service_detected(); // FIXIT-M why is this set here and not when add_service is called? + // FIXIT-M why is this set here and not when add_service is called? + args.asd.set_service_detected(); return APPID_SUCCESS; default: goto bail; diff --git a/src/network_inspectors/appid/service_plugins/service_ssl.cc b/src/network_inspectors/appid/service_plugins/service_ssl.cc index 96ea2a063..bd7ef9491 100644 --- a/src/network_inspectors/appid/service_plugins/service_ssl.cc +++ b/src/network_inspectors/appid/service_plugins/service_ssl.cc @@ -1113,9 +1113,11 @@ bool setSSLSquelch(Packet* p, int type, AppId appId, AppIdInspector& inspector) const SfIp* dip = p->ptrs.ip_api.get_dst(); const SfIp* sip = p->ptrs.ip_api.get_src(); - // FIXIT-H: Passing appId to create_future_session() is incorrect. We need to pass the snort_protocol_id associated with appId. - AppIdSession* asd = AppIdSession::create_future_session(p, sip, 0, dip, p->ptrs.dp, IpProtocol::TCP, - appId, 0, inspector); + // FIXIT-H: Passing appId to create_future_session() is incorrect. We + // need to pass the snort_protocol_id associated with appId. + AppIdSession* asd = AppIdSession::create_future_session( + p, sip, 0, dip, p->ptrs.dp, IpProtocol::TCP, appId, 0, inspector); + if ( asd ) { switch (type) diff --git a/src/network_inspectors/appid/service_plugins/test/service_rsync_test.cc b/src/network_inspectors/appid/service_plugins/test/service_rsync_test.cc index 80772d240..8f6229a92 100644 --- a/src/network_inspectors/appid/service_plugins/test/service_rsync_test.cc +++ b/src/network_inspectors/appid/service_plugins/test/service_rsync_test.cc @@ -19,7 +19,7 @@ // service_rsync_test.cc author Steve Chew // unit test for service_rsync -// FIXIT-M - unit tests disabled until mocking support can be figured out +// FIXIT-RC - unit tests disabled until mocking support can be figured out #ifdef HAVE_CONFIG_H #include "config.h" diff --git a/src/network_inspectors/appid/service_state.cc b/src/network_inspectors/appid/service_state.cc index 847778c1f..d78c6bc85 100644 --- a/src/network_inspectors/appid/service_state.cc +++ b/src/network_inspectors/appid/service_state.cc @@ -329,7 +329,7 @@ void AppIdServiceState::check_reset(AppIdSession& asd, const SfIp* ip, uint16_t else if ( ( packet_time() - sds->get_reset_time() ) >= 60 ) { AppIdServiceState::remove(ip, IpProtocol::TCP, port, asd.is_decrypted()); - // FIXIT-L - Remove if this flag not used anywhere + // FIXIT-RC - Remove if this flag not used anywhere asd.set_session_flags(APPID_SESSION_SERVICE_DELETED); } } @@ -337,7 +337,7 @@ void AppIdServiceState::check_reset(AppIdSession& asd, const SfIp* ip, uint16_t void AppIdServiceState::dump_stats() { - // FIXIT-L - do we need to keep ipv4 and ipv6 separate? + // FIXIT-L - do we need to keep ipv4 and ipv6 separate? CRC: No. #if 0 LogMessage("Service State:\n"); if (serviceStateCache4) diff --git a/src/network_inspectors/appid/tp_appid_utils.cc b/src/network_inspectors/appid/tp_appid_utils.cc index 5a9f64240..8177ae0e6 100644 --- a/src/network_inspectors/appid/tp_appid_utils.cc +++ b/src/network_inspectors/appid/tp_appid_utils.cc @@ -94,6 +94,8 @@ static inline int check_ssl_appid_for_reinspect(AppId app_id) // // Or, register observers with THirdPartyAppIDAttributeData and modify the // set functions to copy the tp buffers directly into the appropriate observer. +// +// Or, replace ThirdParty with 1st Party http_inspect. static inline void process_http_session(AppIdSession& asd, ThirdPartyAppIDAttributeData& attribute_data, AppidChangeBits& change_bits) { diff --git a/src/network_inspectors/perf_monitor/perf_formatter.h b/src/network_inspectors/perf_monitor/perf_formatter.h index 21bb1c606..54c560ef8 100644 --- a/src/network_inspectors/perf_monitor/perf_formatter.h +++ b/src/network_inspectors/perf_monitor/perf_formatter.h @@ -40,7 +40,7 @@ // 5. Call write to output the current values in each field. // // init_output should be implemented where metadata needs to be written on -// ouput open. +// output open. // #include diff --git a/src/network_inspectors/perf_monitor/perf_tracker.cc b/src/network_inspectors/perf_monitor/perf_tracker.cc index 3fe2f1470..d22f61b16 100644 --- a/src/network_inspectors/perf_monitor/perf_tracker.cc +++ b/src/network_inspectors/perf_monitor/perf_tracker.cc @@ -108,12 +108,12 @@ bool PerfTracker::open(bool append) const char* file_name = fname.c_str(); bool existed = false; - /*Check file before change permission*/ + // Check file before change permission if (stat(file_name, &pt) == 0) { existed = true; - /*Only change permission for file owned by root*/ + // Only change permission for file owned by root if ((0 == pt.st_uid) || (0 == pt.st_gid)) { if (chmod(file_name, mode) != 0) diff --git a/src/network_inspectors/port_scan/ps_module.h b/src/network_inspectors/port_scan/ps_module.h index a82668866..6a290b0f4 100644 --- a/src/network_inspectors/port_scan/ps_module.h +++ b/src/network_inspectors/port_scan/ps_module.h @@ -154,9 +154,10 @@ public: PortscanConfig* get_data(); + // FIXIT-M this should eventually be CONTEXT. + // Set to GLOBAL so this isn't selected away when inspection policy switches Usage get_usage() const override - { return GLOBAL; } // FIXIT-M this should eventually be CONTEXT. - // Set to GLOBAL so this isn't selected away when inspection policy switches + { return GLOBAL; } private: PS_ALERT_CONF* get_alert_conf(const char* fqn); diff --git a/src/parser/parse_conf.h b/src/parser/parse_conf.h index b32d5ef1f..1bd91e793 100644 --- a/src/parser/parse_conf.h +++ b/src/parser/parse_conf.h @@ -34,6 +34,7 @@ struct SnortConfig; void ParseConfigFile(snort::SnortConfig*, const char* fname); void ParseConfigString(snort::SnortConfig*, const char* str); +void ParseIpVar(snort::SnortConfig*, const char* name, const char* s); void parse_include(snort::SnortConfig*, const char*); void AddRuleState(snort::SnortConfig*, const RuleState&); diff --git a/src/parser/parse_rule.cc b/src/parser/parse_rule.cc index 7fe0487ff..7a93155c0 100644 --- a/src/parser/parse_rule.cc +++ b/src/parser/parse_rule.cc @@ -599,12 +599,6 @@ static int ParsePortList( { ParseError("Pure NOT ports are not allowed."); return -1; - /* - if( dst_flag ) - rtn->flags |= EXCEPT_DST_PORT; - else - rtn->flags |= EXCEPT_SRC_PORT; - */ } /* @@ -800,27 +794,13 @@ static void SetupRTNFuncList(RuleTreeNode* rtn) } else { - /* Attach the proper port checking function to the function list */ - /* - * the in-line "if's" check to see if the "any" or "not" flags have - * been set so the PortToFunc call can determine which port testing - * function to attach to the list - */ - PortToFunc(rtn, (rtn->flags & ANY_DST_PORT) ? 1 : 0, - (rtn->flags & EXCEPT_DST_PORT) ? 1 : 0, DST); - - /* as above */ - PortToFunc(rtn, (rtn->flags & ANY_SRC_PORT) ? 1 : 0, - (rtn->flags & EXCEPT_SRC_PORT) ? 1 : 0, SRC); - - /* link in the proper IP address detection function */ - AddrToFunc(rtn, SRC); + PortToFunc(rtn, (rtn->flags & ANY_DST_PORT) ? 1 : 0, 0, DST); + PortToFunc(rtn, (rtn->flags & ANY_SRC_PORT) ? 1 : 0, 0, SRC); - /* last verse, same as the first (but for dest IP) ;) */ + AddrToFunc(rtn, SRC); AddrToFunc(rtn, DST); } - /* tack the end (success) function to the list */ AddRuleFuncToList(RuleListEnd, rtn); } diff --git a/src/parser/parser.cc b/src/parser/parser.cc index d28da3b6a..7057648c2 100644 --- a/src/parser/parser.cc +++ b/src/parser/parser.cc @@ -269,14 +269,7 @@ static void DefineIfaceVar(SnortConfig* sc, char* iname, const uint8_t* network, VarDefine(sc, varbuf, valbuf); } -/**************************************************************************** - * - * Function : DefineAllIfaceVars() - * Purpose : Find all up interfaces and define iface_ADDRESS vars for them - * Arguments : none - * Returns : void function - * - ****************************************************************************/ +// Find all up interfaces and define iface_ADDRESS vars for them static void DefineAllIfaceVars(SnortConfig* sc) { diff --git a/src/parser/vars.cc b/src/parser/vars.cc index 8f8481b9c..7d25622b7 100644 --- a/src/parser/vars.cc +++ b/src/parser/vars.cc @@ -594,7 +594,7 @@ VarEntry* VarDefine( else p->id = var_id; -#ifdef XXXXXXX +#if 0 vlen = strlen(value); LogMessage("Var '%s' defined, value len = %d chars", p->name, vlen); @@ -654,7 +654,6 @@ const char* VarSearch(SnortConfig* sc, const char* name) if ((ipvar = sfvt_lookup_var(ip_vartable, name)) != nullptr) return ExpandVars(sc, ipvar->value); - /* XXX Return a string value */ if (PortVarTableFind(portVarTable, name)) return name; @@ -673,24 +672,8 @@ const char* VarSearch(SnortConfig* sc, const char* name) return nullptr; } -/**************************************************************************** - * - * Function: ExpandVars() - * - * Purpose: expand all variables in a string - * - * Arguments: - * SnortConfig * - * The snort config that has the vartables. - * char * - * The name of the variable. - * - * Returns: - * char * - * The expanded string. Note that the string is returned in a - * static variable and most likely needs to be string dup'ed. - * - ***************************************************************************/ + // The expanded string. Note that the string is returned in a + // static variable and most likely needs to be string dup'ed. const char* ExpandVars(SnortConfig* sc, const char* string) { static char estring[ 65536 ]; // FIXIT-L convert this foo to a std::string diff --git a/src/parser/vars.h b/src/parser/vars.h index b531d4a7d..73e05b73d 100644 --- a/src/parser/vars.h +++ b/src/parser/vars.h @@ -64,9 +64,6 @@ struct VarEntry VarEntry* VarDefine(snort::SnortConfig*, const char* name, const char* value); int PortVarDefine(snort::SnortConfig*, const char* name, const char* s); -// FIXIT-L put ParseIpVar() definition and declaration in matching files -void ParseIpVar(snort::SnortConfig*, const char* name, const char* s); - VarEntry* VarAlloc(); void DeleteVars(VarEntry* var_table); void AddVarToTable(snort::SnortConfig*, const char*, const char*); diff --git a/src/piglet/piglet.cc b/src/piglet/piglet.cc index 205fa851e..135eabc62 100644 --- a/src/piglet/piglet.cc +++ b/src/piglet/piglet.cc @@ -35,7 +35,7 @@ namespace Piglet { int main() { - // FIXIT-M allow user selection of output/result functions + // FIXIT-L allow user selection of output/result functions if ( Runner::run_all(verbose_output) ) return 0; diff --git a/src/piglet/piglet_api.h b/src/piglet/piglet_api.h index e90a1e5a4..c69b1b2a9 100644 --- a/src/piglet/piglet_api.h +++ b/src/piglet/piglet_api.h @@ -66,20 +66,12 @@ public: const Api* get_api() { return api; } - std::string get_error() - { return error; } - protected: lua_State* L; std::string target; snort::Module* module; snort::SnortConfig* snort_conf; - std::string error; // FIXIT-L unused - - void set_error(const std::string& s) // FIXIT-L unused - { error = s; } - private: const Api* api; }; diff --git a/src/profiler/profiler.h b/src/profiler/profiler.h index c824687d7..4a08e9a63 100644 --- a/src/profiler/profiler.h +++ b/src/profiler/profiler.h @@ -35,7 +35,7 @@ public: static void register_module(const char*, const char*, snort::Module*); static void register_module(const char*, const char*, snort::get_profile_stats_fn); - // FIXIT-L do we need to call on main thread? + // FIXIT-RC do we need to call on main thread? we should know the answer by now. // call from packet threads, just before thread termination static void consolidate_stats(); static void reset_stats(); diff --git a/src/profiler/rule_profiler.cc b/src/profiler/rule_profiler.cc index 4ce3a7e12..d9582db34 100644 --- a/src/profiler/rule_profiler.cc +++ b/src/profiler/rule_profiler.cc @@ -24,11 +24,11 @@ #include "rule_profiler.h" -//#include -//#include -//#include -//#include -//#include +#include +#include +#include +#include +#include // this include eventually leads to possible issues with std::chrono: // 1. Undefined or garbage value returned to caller (rep count()) diff --git a/src/service_inspectors/CMakeLists.txt b/src/service_inspectors/CMakeLists.txt index 84404eac6..02c6cae2c 100644 --- a/src/service_inspectors/CMakeLists.txt +++ b/src/service_inspectors/CMakeLists.txt @@ -25,8 +25,11 @@ if (STATIC_INSPECTORS) $ $ $ + $ $ + $ $ + $ $ $ ) @@ -35,10 +38,7 @@ endif() set(STATIC_SERVICE_INSPECTOR_PLUGINS $ $ - $ - $ $ - $ $ ${STATIC_INSPECTOR_OBJS} CACHE INTERNAL "STATIC_SERVICE_INSPECTOR_PLUGINS" diff --git a/src/service_inspectors/dce_rpc/dce_co.cc b/src/service_inspectors/dce_rpc/dce_co.cc index fbb222dfc..6494be375 100644 --- a/src/service_inspectors/dce_rpc/dce_co.cc +++ b/src/service_inspectors/dce_rpc/dce_co.cc @@ -224,6 +224,8 @@ static DCE2_Ret DCE2_CoSetIface(DCE2_SsnData* sd, DCE2_CoTracker* cot, uint16_t /* This should be set if we've gotten a Bind */ if (cot->ctx_ids == nullptr) return DCE2_RET__ERROR; + + // FIXIT-M these Profile aren't actually helping ... if (sd->trans == DCE2_TRANS_TYPE__TCP) { Profile profile(dce2_tcp_pstat_co_ctx); @@ -232,8 +234,7 @@ static DCE2_Ret DCE2_CoSetIface(DCE2_SsnData* sd, DCE2_CoTracker* cot, uint16_t { Profile profile(dce2_smb_pstat_co_ctx); } - // FIXIT-M add HTTP, UDP cases when these are ported - // same for all other instances of profiling + // FIXIT-M add missing cases (HTTP, UDP, ...) DCE2_CoCtxIdNode* ctx_id_node = (DCE2_CoCtxIdNode*)DCE2_ListFind(cot->ctx_ids, (void*)(uintptr_t)ctx_id); diff --git a/src/service_inspectors/ftp_telnet/ftp.cc b/src/service_inspectors/ftp_telnet/ftp.cc index 1f5e97939..5caf0f294 100644 --- a/src/service_inspectors/ftp_telnet/ftp.cc +++ b/src/service_inspectors/ftp_telnet/ftp.cc @@ -151,7 +151,7 @@ static int snort_ftp(Packet* p) } else { - /* XXX - Not FTP or Telnet */ + /* Not FTP or Telnet */ assert(false); p->flow->free_flow_data(FtpFlowData::inspector_id); return 0; diff --git a/src/service_inspectors/ftp_telnet/ftp_cmd_lookup.cc b/src/service_inspectors/ftp_telnet/ftp_cmd_lookup.cc index 4d05bf28b..bb2f36dd4 100644 --- a/src/service_inspectors/ftp_telnet/ftp_cmd_lookup.cc +++ b/src/service_inspectors/ftp_telnet/ftp_cmd_lookup.cc @@ -104,26 +104,6 @@ int ftp_cmd_lookup_cleanup(CMD_LOOKUP** CmdLookup) return FTPP_SUCCESS; } -/* - * Function: ftp_cmd_lookup_add(CMD_LOOKUP *CmdLookup, - * char *ip, int len, - * FTP_CMD_CONF *FTPCmd) - * - * Purpose: Add a cmd configuration to the list. - * We add these keys like you would normally think to add - * them, because on low endian machines the least significant - * byte is compared first. This is what we want to compare - * IPs backward, doesn't work on high endian machines, but oh - * well. Our platform is Intel. FIXIT-L say what? endian madness - * - * Arguments: CmdLookup => a pointer to the lookup structure - * cmd => the ftp cmd - * len => Length of the cmd - * FTPCmd => a pointer to the cmd configuration structure - * - * Returns: int => return code indicating error or success - * - */ int ftp_cmd_lookup_add(CMD_LOOKUP* CmdLookup, const char* cmd, int len, FTP_CMD_CONF* FTPCmd) { diff --git a/src/service_inspectors/ftp_telnet/ftpdata_splitter.cc b/src/service_inspectors/ftp_telnet/ftpdata_splitter.cc index 31d6405f2..71c66f3b5 100644 --- a/src/service_inspectors/ftp_telnet/ftpdata_splitter.cc +++ b/src/service_inspectors/ftp_telnet/ftpdata_splitter.cc @@ -51,9 +51,8 @@ StreamSplitter::Status FtpDataSplitter::scan(Flow* flow, const uint8_t*, uint32_ if(expected_seg_size == 0) { // FIXIT-M: Can we do better than this guess if no MSS is specified? - // Malware detection won't work if expected_seg_size - // doesn't match the payload lengths on packets before - // the last packet. + // Malware detection won't work if expected_seg_size doesn't match + // the payload lengths on packets before the last packet. expected_seg_size = 1448; if(flow->session and flow->pkt_type == PktType::TCP) diff --git a/src/service_inspectors/http2_inspect/http2_enum.h b/src/service_inspectors/http2_inspect/http2_enum.h index 6cbac02c5..6413112c1 100644 --- a/src/service_inspectors/http2_inspect/http2_enum.h +++ b/src/service_inspectors/http2_inspect/http2_enum.h @@ -28,7 +28,7 @@ static const int MAX_OCTETS = 63780; static const int DATA_SECTION_SIZE = 16384; static const int FRAME_HEADER_LENGTH = 9; -// FIXIT-M need to replace with a real number +// FIXIT-RC need to replace with a real number. CRC: use 120. static const uint32_t HTTP2_GID = 219; // Message originator--client or server diff --git a/src/service_inspectors/http_inspect/http_js_norm.cc b/src/service_inspectors/http_inspect/http_js_norm.cc index 7ed3d5441..ef2833f6a 100644 --- a/src/service_inspectors/http_inspect/http_js_norm.cc +++ b/src/service_inspectors/http_inspect/http_js_norm.cc @@ -145,6 +145,7 @@ void HttpJsNorm::normalize(const Field& input, Field& output, HttpInfractions* i JSNormalizeDecode(js_start, (uint16_t)(end-js_start), (char*)buffer+index, (uint16_t)(input.length() - index), &ptr, &bytes_copied, &js, uri_param.iis_unicode ? uri_param.unicode_map : nullptr); + index += bytes_copied; } else diff --git a/src/service_inspectors/http_inspect/http_msg_head_shared.h b/src/service_inspectors/http_inspect/http_msg_head_shared.h index 13c0c3009..8764800f0 100644 --- a/src/service_inspectors/http_inspect/http_msg_head_shared.h +++ b/src/service_inspectors/http_inspect/http_msg_head_shared.h @@ -86,7 +86,7 @@ private: static const HeaderNormalizer* const header_norms[]; // All of these are indexed by the relative position of the header field in the message - static const int MAX_HEADERS = 200; // I'm an arbitrary number. FIXIT-L + static const int MAX_HEADERS = 200; // I'm an arbitrary number. FIXIT-RC static const int MAX_HEADER_LENGTH = 4096; // Based on max cookie size of some browsers void parse_header_block(); diff --git a/src/service_inspectors/http_inspect/http_str_to_code.cc b/src/service_inspectors/http_inspect/http_str_to_code.cc index a4d2ebf31..da88574b3 100644 --- a/src/service_inspectors/http_inspect/http_str_to_code.cc +++ b/src/service_inspectors/http_inspect/http_str_to_code.cc @@ -32,8 +32,8 @@ int32_t str_to_code(const uint8_t* text, const int32_t text_len, const StrCode t { for (int32_t k=0; table[k].name != nullptr; k++) { - if ((text_len == (int)strlen(table[k].name)) && (memcmp(text, table[k].name, text_len) == - 0)) + if ((text_len == (int)strlen(table[k].name)) && + (memcmp(text, table[k].name, text_len) == 0)) { return table[k].code; } diff --git a/src/service_inspectors/http_inspect/http_tables.cc b/src/service_inspectors/http_inspect/http_tables.cc index cf2cd3505..fbdc28317 100644 --- a/src/service_inspectors/http_inspect/http_tables.cc +++ b/src/service_inspectors/http_inspect/http_tables.cc @@ -215,7 +215,7 @@ const HeaderNormalizer HttpMsgHeadShared::NORMALIZER_CHARSET #if defined(__clang__) // Designated initializers are not supported in C++11. However we're going to play compilation -// roulette and hopes this works. +// roulette and hope this works. #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wc99-extensions" #endif diff --git a/src/service_inspectors/imap/CMakeLists.txt b/src/service_inspectors/imap/CMakeLists.txt index e248e20fd..b0db57973 100644 --- a/src/service_inspectors/imap/CMakeLists.txt +++ b/src/service_inspectors/imap/CMakeLists.txt @@ -9,12 +9,11 @@ set( FILE_LIST imap_module.h ) -# can't be be linked dynamically yet -#if (STATIC_INSPECTORS) +if (STATIC_INSPECTORS) add_library( imap OBJECT ${FILE_LIST}) -#else (STATIC_INSPECTORS) - #add_dynamic_module(imap inspectors ${FILE_LIST}) +else (STATIC_INSPECTORS) + add_dynamic_module(imap inspectors ${FILE_LIST}) -#endif (STATIC_INSPECTORS) +endif (STATIC_INSPECTORS) diff --git a/src/service_inspectors/imap/imap.cc b/src/service_inspectors/imap/imap.cc index e9a153a79..412a5c068 100644 --- a/src/service_inspectors/imap/imap.cc +++ b/src/service_inspectors/imap/imap.cc @@ -811,8 +811,6 @@ const InspectApi imap_api = nullptr // reset }; -#undef BUILDING_SO // FIXIT-L can't be linked dynamically yet - #ifdef BUILDING_SO SO_PUBLIC const BaseApi* snort_plugins[] = { diff --git a/src/service_inspectors/imap/imap_config.h b/src/service_inspectors/imap/imap_config.h index 1f5d1e08e..57737e86b 100644 --- a/src/service_inspectors/imap/imap_config.h +++ b/src/service_inspectors/imap/imap_config.h @@ -35,7 +35,7 @@ struct ImapStats PegCount sessions; PegCount concurrent_sessions; PegCount max_concurrent_sessions; - MimeStats mime_stats; + snort::MimeStats mime_stats; }; extern const PegInfo imap_peg_names[]; diff --git a/src/service_inspectors/pop/CMakeLists.txt b/src/service_inspectors/pop/CMakeLists.txt index f9b08865d..6c8a153dd 100644 --- a/src/service_inspectors/pop/CMakeLists.txt +++ b/src/service_inspectors/pop/CMakeLists.txt @@ -9,12 +9,11 @@ set( FILE_LIST pop_module.h ) -# can't be be linked dynamically yet -#if (STATIC_INSPECTORS) +if (STATIC_INSPECTORS) add_library( pop OBJECT ${FILE_LIST}) -#else (STATIC_INSPECTORS) - #add_dynamic_module(pop inspectors ${FILE_LIST}) +else (STATIC_INSPECTORS) + add_dynamic_module(pop inspectors ${FILE_LIST}) -#endif (STATIC_INSPECTORS) +endif (STATIC_INSPECTORS) diff --git a/src/service_inspectors/pop/pop.cc b/src/service_inspectors/pop/pop.cc index 14f45a0d3..d76b6cffb 100644 --- a/src/service_inspectors/pop/pop.cc +++ b/src/service_inspectors/pop/pop.cc @@ -750,8 +750,6 @@ const InspectApi pop_api = nullptr // reset }; -#undef BUILDING_SO // FIXIT-L can't be linked dynamically yet - #ifdef BUILDING_SO SO_PUBLIC const BaseApi* snort_plugins[] = { diff --git a/src/service_inspectors/pop/pop_config.h b/src/service_inspectors/pop/pop_config.h index 133c9a705..44ad12ff5 100644 --- a/src/service_inspectors/pop/pop_config.h +++ b/src/service_inspectors/pop/pop_config.h @@ -35,7 +35,7 @@ struct PopStats PegCount sessions; PegCount concurrent_sessions; PegCount max_concurrent_sessions; - MimeStats mime_stats; + snort::MimeStats mime_stats; }; extern const PegInfo pop_peg_names[]; diff --git a/src/service_inspectors/service_inspectors.cc b/src/service_inspectors/service_inspectors.cc index 029bcbcd8..e90bed4d2 100644 --- a/src/service_inspectors/service_inspectors.cc +++ b/src/service_inspectors/service_inspectors.cc @@ -27,10 +27,6 @@ using namespace snort; -extern const BaseApi* sin_imap; -extern const BaseApi* sin_pop; -extern const BaseApi* sin_smtp; - extern const BaseApi* sin_file[]; extern const BaseApi* sin_http[]; extern const BaseApi* sin_http2[]; @@ -43,11 +39,15 @@ extern const BaseApi* sin_dns; extern const BaseApi* sin_ftp_client; extern const BaseApi* sin_ftp_server; extern const BaseApi* sin_ftp_data; +extern const BaseApi* sin_imap; +extern const BaseApi* sin_pop; extern const BaseApi* sin_rpc_decode; +extern const BaseApi* sin_smtp; extern const BaseApi* sin_ssh; extern const BaseApi* sin_telnet; extern const BaseApi* sin_wizard; +// these define multiple plugins extern const BaseApi* sin_dce[]; extern const BaseApi* sin_dnp3[]; extern const BaseApi* sin_gtp[]; @@ -56,17 +56,16 @@ extern const BaseApi* sin_modbus[]; const BaseApi* service_inspectors[] = { - sin_imap, - sin_pop, - sin_smtp, - #ifdef STATIC_INSPECTORS sin_bo, sin_dns, sin_ftp_client, sin_ftp_server, sin_ftp_data, + sin_imap, + sin_pop, sin_rpc_decode, + sin_smtp, sin_ssh, sin_telnet, sin_wizard, diff --git a/src/service_inspectors/sip/ips_sip_method.cc b/src/service_inspectors/sip/ips_sip_method.cc index 2261083df..0764377c2 100644 --- a/src/service_inspectors/sip/ips_sip_method.cc +++ b/src/service_inspectors/sip/ips_sip_method.cc @@ -121,8 +121,8 @@ IpsOption::EvalStatus SipMethodOption::eval(Cursor&, Packet* p) if ( !ropts->method_data ) return NO_MATCH; - //FIXIT-P This should really be evaluated once per request instead of once - //per rule option evaluation. + // FIXIT-P This should really be evaluated once per request instead of once + // per rule option evaluation. std::string method(ropts->method_data, ropts->method_len); std::transform(method.begin(), method.end(), method.begin(), ::toupper); diff --git a/src/service_inspectors/smtp/CMakeLists.txt b/src/service_inspectors/smtp/CMakeLists.txt index 2110aa60f..aefde10df 100644 --- a/src/service_inspectors/smtp/CMakeLists.txt +++ b/src/service_inspectors/smtp/CMakeLists.txt @@ -15,12 +15,11 @@ set( FILE_LIST smtp_normalize.h ) -# can't be be linked dynamically yet -#if (STATIC_INSPECTORS) +if (STATIC_INSPECTORS) add_library( smtp OBJECT ${FILE_LIST}) -#else (STATIC_INSPECTORS) - #add_dynamic_module(smtp inspectors ${FILE_LIST}) +else (STATIC_INSPECTORS) + add_dynamic_module(smtp inspectors ${FILE_LIST}) -#endif (STATIC_INSPECTORS) +endif (STATIC_INSPECTORS) diff --git a/src/service_inspectors/smtp/smtp.cc b/src/service_inspectors/smtp/smtp.cc index 376d53ef0..c7bd45386 100644 --- a/src/service_inspectors/smtp/smtp.cc +++ b/src/service_inspectors/smtp/smtp.cc @@ -1316,7 +1316,7 @@ int SmtpMime::handle_header_line( } - /* XXX Does VRT want data headers normalized? + /* Does VRT want data headers normalized? * currently the code does not normalize headers */ if (smtp_normalizing) { @@ -1575,8 +1575,6 @@ const InspectApi smtp_api = nullptr // reset }; -#undef BUILDING_SO // FIXIT-L can't be linked dynamically yet - #ifdef BUILDING_SO SO_PUBLIC const BaseApi* snort_plugins[] = { diff --git a/src/service_inspectors/smtp/smtp_config.h b/src/service_inspectors/smtp/smtp_config.h index 3546a72a6..888ee77d1 100644 --- a/src/service_inspectors/smtp/smtp_config.h +++ b/src/service_inspectors/smtp/smtp_config.h @@ -150,7 +150,7 @@ struct SmtpStats PegCount sessions; PegCount concurrent_sessions; PegCount max_concurrent_sessions; - MimeStats mime_stats; + snort::MimeStats mime_stats; }; extern const PegInfo smtp_peg_names[]; diff --git a/src/service_inspectors/smtp/smtp_normalize.cc b/src/service_inspectors/smtp/smtp_normalize.cc index 8f91f05ea..04a803952 100644 --- a/src/service_inspectors/smtp/smtp_normalize.cc +++ b/src/service_inspectors/smtp/smtp_normalize.cc @@ -47,14 +47,15 @@ * If command doesn't need normalizing it will do nothing, except in * the case where we are already normalizing in which case the line * will get copied to the alt buffer. + * * If the command needs normalizing the normalized data will be copied * to the alt buffer. If we are not already normalizing, all of the * data up to this point will be copied into the alt buffer first. * - * XXX This may copy unwanted data if we are ignoring the data in the - * message and there was data that came before the command in the - * packet, for example if there are multiple transactions on the - * session or if we're normalizing QUIT. + * This may copy unwanted data if we are ignoring the data in the + * message and there was data that came before the command in the + * packet, for example if there are multiple transactions on the + * session or if we're normalizing QUIT. * * @param p pointer to packet structure * @param ptr pointer to beginning of command line diff --git a/src/service_inspectors/smtp/smtp_util.cc b/src/service_inspectors/smtp/smtp_util.cc index 03ec077fb..4c9570a3d 100644 --- a/src/service_inspectors/smtp/smtp_util.cc +++ b/src/service_inspectors/smtp/smtp_util.cc @@ -38,15 +38,11 @@ using namespace snort; void SMTP_GetEOL(const uint8_t* ptr, const uint8_t* end, const uint8_t** eol, const uint8_t** eolm) { - const uint8_t* tmp_eol; - const uint8_t* tmp_eolm; + assert(ptr and end and eol and eolm); - /* XXX maybe should fatal error here since none of these - * pointers should be NULL */ - if (ptr == nullptr || end == nullptr || eol == nullptr || eolm == nullptr) - return; + const uint8_t* tmp_eolm; + const uint8_t* tmp_eol = (uint8_t*)memchr(ptr, '\n', end - ptr); - tmp_eol = (uint8_t*)memchr(ptr, '\n', end - ptr); if (tmp_eol == nullptr) { tmp_eol = end; diff --git a/src/sfip/sf_cidr.cc b/src/sfip/sf_cidr.cc index 0eda6d4d5..784c4561c 100644 --- a/src/sfip/sf_cidr.cc +++ b/src/sfip/sf_cidr.cc @@ -31,10 +31,10 @@ SfIpRet SfCidr::set(const char* src) return addr.set(src, &bits); } -/* Check if ip is contained within the network specified by this addr */ -/* Returns SFIP_EQUAL if so. - * XXX assumes that "ip" is not less specific than "addr" XXX -*/ +// Check if ip is contained within the network specified by this addr +// Returns SFIP_EQUAL if so. +// assumes that "ip" is not less specific than "addr" + SfIpRet SfCidr::contains(const SfIp* ip) const { uint16_t i; diff --git a/src/sfip/sf_ip.cc b/src/sfip/sf_ip.cc index 318bc93b5..143a5695b 100644 --- a/src/sfip/sf_ip.cc +++ b/src/sfip/sf_ip.cc @@ -159,10 +159,8 @@ static inline int _netmask_str_to_bit_count(char* mask, int family) int bits, i, nBits, nBytes; uint8_t* bytes = (uint8_t*)buf; - /* XXX - * Mask not validated. - * Only sfip_pton should be using this function, and using it safely. - * XXX */ + // Mask not validated. + // Only sfip_pton should be using this function, and using it safely. if (inet_pton(family, mask, buf) < 1) return -1; diff --git a/src/sfip/sf_ip.h b/src/sfip/sf_ip.h index 00f0bad2a..030632b10 100644 --- a/src/sfip/sf_ip.h +++ b/src/sfip/sf_ip.h @@ -469,7 +469,8 @@ inline std::ostream& operator<<(std::ostream& os, const SfIp* addr) return os << addr->ntop(str); } -// FIXIT-L X This should be in utils_net if anywhere, but that makes it way harder to link into unit tests +// FIXIT-L X This should be in utils_net if anywhere, but that makes it way +// harder to link into unit tests SO_PUBLIC const char* snort_inet_ntop(int family, const void* ip_raw, char* buf, int bufsize); } // namespace snort #endif diff --git a/src/sfip/sf_ipvar.cc b/src/sfip/sf_ipvar.cc index 4cda39f08..0d0648319 100644 --- a/src/sfip/sf_ipvar.cc +++ b/src/sfip/sf_ipvar.cc @@ -74,7 +74,7 @@ void sfvar_free(sfip_var_t* var) } else if (var->mode == SFIP_TABLE) { - // XXX + // FIXIT-L SFIP_TABLE free unimplemented } snort_free(var); @@ -348,9 +348,9 @@ static SfIpRet sfvar_add(sfip_var_t* dst, sfip_var_t* src) return SFIP_SUCCESS; } -/* Adds the nodes in 'src' to the variable 'dst' */ -/* The mismatch of types is for ease-of-supporting Snort4 and - * Snort6 simultaneously */ +// Adds the nodes in 'src' to the variable 'dst' +// The mismatch of types is for ease-of-supporting Snort4 and +// Snort6 simultaneously static SfIpRet sfvar_add_node(sfip_var_t* var, sfip_node_t* node, int negated) { sfip_node_t* p; @@ -361,9 +361,8 @@ static SfIpRet sfvar_add_node(sfip_var_t* var, sfip_node_t* node, int negated) if (!var || !node) return SFIP_ARG_ERR; - /* XXX */ - /* As of this writing, 11/20/06, nodes are always added to - * the list, regardless of the mode (list or table). */ + // As of this writing, 11/20/06, nodes are always added to + // the list, regardless of the mode (list or table). if (negated) { @@ -466,8 +465,8 @@ static SfIpRet sfvar_add_node(sfip_var_t* var, sfip_node_t* node, int negated) return SFIP_SUCCESS; - /* XXX Insert new node into routing table */ -// sfrt_add(node->ip, + // FIXIT-L Insert new node into routing table + // sfrt_add(node->ip, } sfip_var_t* sfvar_create_alias(const sfip_var_t* alias_from, const char* alias_to) diff --git a/src/sfip/sf_ipvar.h b/src/sfip/sf_ipvar.h index 7e7f4d15f..ee73913c4 100644 --- a/src/sfip/sf_ipvar.h +++ b/src/sfip/sf_ipvar.h @@ -58,7 +58,6 @@ typedef struct _ip_node #define ip_addr ip; /* To ease porting Snort */ struct _ip_node* next; int flags; - // XXX int addr_flags; /* Flags used exclusively by Snort */ /* Keeping these variables separate keeps * this from stepping on Snort's toes. */ diff --git a/src/sfip/sf_vartable.cc b/src/sfip/sf_vartable.cc index 98d2de4f8..0772ea269 100644 --- a/src/sfip/sf_vartable.cc +++ b/src/sfip/sf_vartable.cc @@ -181,7 +181,7 @@ sfvt_expand_value_error: return nullptr; } -// XXX this implementation is just used to support +// this implementation is just used to support // Snort's underlying implementation better SfIpRet sfvt_define(vartable_t* table, const char* name, const char* value) { @@ -321,12 +321,10 @@ sfip_var_t* sfvt_lookup_var(vartable_t* table, const char* name) if (*name == '$') name++; - /* XXX should I assume there will be trailing garbage or + /* should I assume there will be trailing garbage or * should I automatically find where the variable ends? */ - for (end=name; - *end && !isspace((int)*end) && *end != '\\' && *end != ']'; - end++) - ; + for (end=name; *end && !isspace((int)*end) && *end != '\\' && *end != ']'; end++); + len = end - name; for (p=table->head; len && p; p=p->next) diff --git a/src/sfrt/sfrt.cc b/src/sfrt/sfrt.cc index ad12622f3..57daae99f 100644 --- a/src/sfrt/sfrt.cc +++ b/src/sfrt/sfrt.cc @@ -359,8 +359,7 @@ GENERIC sfrt_search(const SfIp* ip, unsigned char len, table_t* table) else return nullptr; - /* FIXIT-M - Is is true that we don't support v6 yet? */ - /* IPv6 not yet supported */ + // FIXIT-RC IPv6 not yet supported by sfrt? if (table->ip_type == IPv6) return nullptr; diff --git a/src/stream/base/stream_base.cc b/src/stream/base/stream_base.cc index d4d182c12..35f08c639 100644 --- a/src/stream/base/stream_base.cc +++ b/src/stream/base/stream_base.cc @@ -25,6 +25,7 @@ #include "flow/flow_control.h" #include "flow/prune_stats.h" #include "main/snort_config.h" +#include "main/snort_types.h" #include "managers/inspector_manager.h" #include "profiler/profiler_defs.h" #include "protocols/packet.h" @@ -120,15 +121,12 @@ void base_reset() static inline bool is_eligible(Packet* p) { - // FIXIT-M extra check? bad checksums should be removed in detect.c snort_inspect() - if ( p->ptrs.decode_flags & DECODE_ERR_CKSUM_IP ) - return false; - - if ( p->packet_flags & PKT_REBUILT_STREAM ) - return false; - - if ( !p->ptrs.ip_api.is_valid() ) - return false; +#ifdef NDEBUG + UNUSED(p); +#endif + assert(!(p->ptrs.decode_flags & DECODE_ERR_CKSUM_IP)); + assert(!(p->packet_flags & PKT_REBUILT_STREAM)); + assert(p->ptrs.ip_api.is_valid()); return true; } diff --git a/src/stream/ip/ip_defrag.cc b/src/stream/ip/ip_defrag.cc index 49f071c2a..4fc48cb58 100644 --- a/src/stream/ip/ip_defrag.cc +++ b/src/stream/ip/ip_defrag.cc @@ -372,7 +372,7 @@ static inline int FragCheckFirstLast( * offset of the new last frag to immediately * after the existing last frag. */ - /* XXX: how to handle that case? punt? */ + /* how to handle that case? punt? */ retVal = FRAG_LAST_OFFSET_ADJUST; } break; @@ -456,10 +456,9 @@ static int FragHandleIPOptions( { /* check that options match those from other non-offset 0 packets */ - /* XXX: could check each individual option here, but that - * would be performance ugly. So, we'll just check that the - * option sizes match. Alert if invalid, but still include in - * reassembly. + /* could check each individual option here, but that would be + * performance ugly. So, we'll just check that the option sizes + * match. Alert if invalid, but still include in reassembly. */ if (ft->copied_ip_options_len) { @@ -635,8 +634,8 @@ static void FragRebuild(FragTracker* ft, Packet* p) } else if (ft->copied_ip_options_len) { - /* XXX: should we log a warning here? there were IP options - * copied across all fragments, EXCEPT the offset 0 fragment. + /* should we log a warning here? there were IP options copied + * across all fragments, EXCEPT the offset 0 fragment. */ } @@ -703,7 +702,7 @@ static void FragRebuild(FragTracker* ft, Packet* p) { if ( !p->is_ip6() ) { - /*XXX: Log message, failed to copy */ + /* Log message, failed to copy */ ft->frag_flags = ft->frag_flags | FRAG_REBUILT; return; } @@ -1899,7 +1898,7 @@ int Defrag::new_tracker(Packet* p, FragTracker* ft) /* insert the fragment into the frag list */ ft->fraglist = f; ft->fraglist_tail = f; - ft->fraglist_count = 1; /* XXX: Are these duplicates? */ + ft->fraglist_count = 1; /* Are these duplicates? */ ft->frag_pkts = 1; /* diff --git a/src/stream/libtcp/stream_tcp_unit_test.cc b/src/stream/libtcp/stream_tcp_unit_test.cc index a8661e11a..8458541cb 100644 --- a/src/stream/libtcp/stream_tcp_unit_test.cc +++ b/src/stream/libtcp/stream_tcp_unit_test.cc @@ -40,37 +40,37 @@ using namespace snort; // SYN PACKET // IP 192.168.0.89.9012 > p3nlh044.shr.prod.phx3.secureserver.net.http: Flags [S], seq 9050, win // 8192, length 0 -uint8_t cooked_syn[] = +static const uint8_t cooked_syn[] = "\x00\x21\x91\x01\xb2\x48\xaa\x00\x04\x00\x0a\x04\x08\x00\x45\x00\x00\x28\x00\x01\x00\x00\x40\x06\x88\x96\xc0\xa8\x00\x59\x48\xa7\xe8\x90\x23\x34\x00\x50\x00\x00\x23\x5a\x00\x00\x00\x00\x50\x02\x20\x00\x56\xcb\x00\x00"; // SYN-ACK PACKET // IP p3nlh044.shr.prod.phx3.secureserver.net.http > 192.168.0.89.9012: Flags [S.], seq 9025, ack // 9051, win 8192, length 0 -uint8_t cooked_syn_ack[] = +static const uint8_t cooked_syn_ack[] = "\xff\xff\xff\xff\xff\xff\x00\x00\x00\x00\x00\x00\x08\x00\x45\x00\x00\x28\x00\x01\x00\x00\x40\x06\x88\x96\x48\xa7\xe8\x90\xc0\xa8\x00\x59\x00\x50\x23\x34\x00\x00\x23\x41\x00\x00\x23\x5b\x50\x12\x20\x00\x33\x79\x00\x00"; // ACK PACKET // IP 192.168.0.89.9012 > p3nlh044.shr.prod.phx3.secureserver.net.http: Flags [.], ack 1, win 8192, // length 0 -uint8_t cooked_ack[] = +static const uint8_t cooked_ack[] = "\x00\x21\x91\x01\xb2\x48\xaa\x00\x04\x00\x0a\x04\x08\x00\x45\x00\x00\x28\x00\x01\x00\x00\x40\x06\x88\x96\xc0\xa8\x00\x59\x48\xa7\xe8\x90\x23\x34\x00\x50\x00\x00\x23\x5b\x00\x00\x23\x42\x50\x10\x20\x00\x33\x7a\x00\x00"; // FIXIT-H this is not a FIN PACKET yet... // IP 192.168.0.89.9012 > p3nlh044.shr.prod.phx3.secureserver.net.http: Flags [.], ack 1, win 8192, // length 0 -uint8_t cooked_fin[] = +static const uint8_t cooked_fin[] = "\x00\x21\x91\x01\xb2\x48\xaa\x00\x04\x00\x0a\x04\x08\x00\x45\x00\x00\x28\x00\x01\x00\x00\x40\x06\x88\x96\xc0\xa8\x00\x59\x48\xa7\xe8\x90\x23\x34\x00\x50\x00\x00\x23\x5b\x00\x00\x23\x42\x50\x10\x20\x00\x33\x7a\x00\x00"; // FIXIT-H this is not a RST PACKET yet... // IP 192.168.0.89.9012 > p3nlh044.shr.prod.phx3.secureserver.net.http: Flags [.], ack 1, win 8192, // length 0 -uint8_t cooked_rst[] = +static const uint8_t cooked_rst[] = "\x00\x21\x91\x01\xb2\x48\xaa\x00\x04\x00\x0a\x04\x08\x00\x45\x00\x00\x28\x00\x01\x00\x00\x40\x06\x88\x96\xc0\xa8\x00\x59\x48\xa7\xe8\x90\x23\x34\x00\x50\x00\x00\x23\x5b\x00\x00\x23\x42\x50\x10\x20\x00\x33\x7a\x00\x00"; // DATA PACKET // IP 192.168.0.89.9012 > p3nlh044.shr.prod.phx3.secureserver.net.http: Flags [P.], seq 1:43, ack // 1, win 8192, length 42 -uint8_t cooked_data[] = +static const uint8_t cooked_data[] = "\x00\x21\x91\x01\xb2\x48\xaa\x00\x04\x00\x0a\x04\x08\x00\x45\x00\x00\x52\x00\x01\x00\x00\x40\x06\x88\x6c\xc0\xa8\x00\x59\x48\xa7\xe8\x90\x23\x34\x00\x50\x00\x00\x23\x5b\x00\x00\x23\x42\x50\x18\x20\x00\x14\x83\x00\x00\x47\x45\x54\x20\x2f\x20\x48\x54\x54\x50\x2f\x31\x2e\x31\x0d\x0a\x48\x6f\x73\x74\x3a\x20\x77\x77\x77\x2e\x6d\x61\x6c\x66\x6f\x72\x67\x65\x2e\x63\x6f\x6d\x0d\x0a\x0d\x0a"; DAQ_PktHdr_t daqHdr; diff --git a/src/stream/libtcp/tcp_state_handler.cc b/src/stream/libtcp/tcp_state_handler.cc index 92b8adcd7..d70adfa4d 100644 --- a/src/stream/libtcp/tcp_state_handler.cc +++ b/src/stream/libtcp/tcp_state_handler.cc @@ -106,7 +106,7 @@ bool TcpStateHandler::eval(TcpSegmentDescriptor& tsd, TcpStreamTracker& tracker) return false; } -// FIXIT-H get the unit test working again +// FIXIT-RC get the unit test working again #ifdef UNIT_TEST_FOO SCENARIO("TCP State Handler Base Class", "[state_handlers][stream_tcp]") diff --git a/src/stream/libtcp/tcp_stream_session.h b/src/stream/libtcp/tcp_stream_session.h index b106c4df9..bfa1b484a 100644 --- a/src/stream/libtcp/tcp_stream_session.h +++ b/src/stream/libtcp/tcp_stream_session.h @@ -73,7 +73,7 @@ public: virtual void clear_session( bool free_flow_data, bool flush_segments, bool restart, snort::Packet* p = nullptr) = 0; - // FIXIT-L these 2 function names convey no meaning afaict... figure out + // FIXIT-RC these 2 function names convey no meaning afaict... figure out // why are they called and name appropriately... virtual void retransmit_process(snort::Packet* p) { diff --git a/src/stream/libtcp/tcp_stream_tracker.cc b/src/stream/libtcp/tcp_stream_tracker.cc index f87b093aa..b7f08fbae 100644 --- a/src/stream/libtcp/tcp_stream_tracker.cc +++ b/src/stream/libtcp/tcp_stream_tracker.cc @@ -240,12 +240,6 @@ void TcpStreamTracker::set_splitter(const Flow* flow) set_splitter(new AtomSplitter(!client_tracker) ); } -void TcpStreamTracker::reset_splitter( ) -{ - if ( splitter ) - splitter->reset(); -} - void TcpStreamTracker::init_on_syn_sent(TcpSegmentDescriptor& tsd) { DeepProfile profile(s5TcpNewSessPerfStats); diff --git a/src/stream/libtcp/tcp_stream_tracker.h b/src/stream/libtcp/tcp_stream_tracker.h index 77813ec26..28e4b9fca 100644 --- a/src/stream/libtcp/tcp_stream_tracker.h +++ b/src/stream/libtcp/tcp_stream_tracker.h @@ -255,7 +255,6 @@ public: virtual void init_flush_policy(); virtual void set_splitter(snort::StreamSplitter* ss); virtual void set_splitter(const snort::Flow* flow); - virtual void reset_splitter( ); virtual void init_on_syn_sent(TcpSegmentDescriptor&); virtual void init_on_syn_recv(TcpSegmentDescriptor&); diff --git a/src/stream/stream_splitter.h b/src/stream/stream_splitter.h index 431d62122..63d0bbd48 100644 --- a/src/stream/stream_splitter.h +++ b/src/stream/stream_splitter.h @@ -80,9 +80,6 @@ public: virtual bool is_paf() { return false; } virtual unsigned max(Flow*); - // FIXIT-L reset is not currently used and may not be needed at all. - // determine if this is so and remove if possible - virtual void reset() { } virtual void update() { } unsigned get_max_pdu() { return max_pdu; } @@ -107,9 +104,11 @@ public: AtomSplitter(bool, uint16_t size = 0); Status scan(Flow*, const uint8_t*, uint32_t, uint32_t, uint32_t*) override; - void reset() override; void update() override; +private: + void reset(); + private: uint16_t base; uint16_t min; @@ -138,10 +137,11 @@ public: Status scan(Flow*, const uint8_t*, uint32_t, uint32_t, uint32_t*) override; +private: bool saw_data() { return byte_count > 0; } - void reset() override + void reset() { byte_count = 0; } private: diff --git a/src/stream/tcp/segment_overlap_editor.cc b/src/stream/tcp/segment_overlap_editor.cc index a7479edf5..97a85ea2e 100644 --- a/src/stream/tcp/segment_overlap_editor.cc +++ b/src/stream/tcp/segment_overlap_editor.cc @@ -142,8 +142,8 @@ int SegmentOverlapEditor::eval_right(TcpReassemblerState& trs) assert(SEQ_LEQ(trs.sos.seq, trs.sos.right->i_seq)); trs.sos.overlap = ( int )( trs.sos.seq_end - trs.sos.right->i_seq ); - // Treat sequence number overlap as a retransmission, only check right side since - // left side happens rarely + // Treat sequence number overlap as a retransmission, + // only check right side since left side happens rarely trs.sos.session->retransmit_handle(trs.sos.tsd->get_pkt()); if ( trs.sos.overlap < trs.sos.right->i_len ) diff --git a/src/stream/tcp/segment_overlap_editor.h b/src/stream/tcp/segment_overlap_editor.h index 89b802c87..41f9fc98b 100644 --- a/src/stream/tcp/segment_overlap_editor.h +++ b/src/stream/tcp/segment_overlap_editor.h @@ -28,7 +28,7 @@ class TcpSession; class TcpStreamTracker; -#define STREAM_INSERT_OK 0 // FIXIT-L replace with bool +#define STREAM_INSERT_OK 0 // FIXIT-RC replace with bool CRC: if useful else just delete struct SegmentOverlapState { diff --git a/src/utils/util_cstring.h b/src/utils/util_cstring.h index 6f0349345..134885ce9 100644 --- a/src/utils/util_cstring.h +++ b/src/utils/util_cstring.h @@ -109,7 +109,7 @@ inline int SnortStrToU32(const char* buffer, char** endptr, buffer++; // If all spaces or a negative sign is found, return error. - // XXX May also want to exclude '+' as well. + // May want to exclude '+' as well. if ((*buffer == '\0') || (*buffer == '-')) return -1; diff --git a/src/utils/util_jsnorm.cc b/src/utils/util_jsnorm.cc index 50ddbd09f..13f9c2915 100644 --- a/src/utils/util_jsnorm.cc +++ b/src/utils/util_jsnorm.cc @@ -637,7 +637,6 @@ static int PNormDecode(char* src, uint16_t srclen, char* dst, uint16_t dstlen, u ptr++; } - //dst = s.output.data; FIXIT-L dead store; should be? *bytes_copied = s.output.len; return iRet; @@ -1277,7 +1276,6 @@ int JSNormalizeDecode(const char* src, uint16_t srclen, char* dst, uint16_t dest (*ptr)++; } - //dst = s.dest.data; FIXIT-L dead store; should be? *bytes_copied = s.dest.len; return RET_OK; diff --git a/tools/snort2lua/config_states/config_binding.cc b/tools/snort2lua/config_states/config_binding.cc index c3304fd10..6618cfd99 100644 --- a/tools/snort2lua/config_states/config_binding.cc +++ b/tools/snort2lua/config_states/config_binding.cc @@ -185,8 +185,8 @@ bool Binding::convert(std::istringstream& data_stream) { Converter bind_cv; - // This will ensure that the final ouput file contains - // lua syntax - even if their are only rules in the file + // This will ensure that the final output file contains + // lua syntax - even if there are only rules in the file bind_cv.get_table_api().open_top_level_table("ips"); bind_cv.get_table_api().close_table(); diff --git a/tools/snort2lua/preprocessor_states/pps_dcerpc_server.cc b/tools/snort2lua/preprocessor_states/pps_dcerpc_server.cc index 91e386d13..0f74cb425 100644 --- a/tools/snort2lua/preprocessor_states/pps_dcerpc_server.cc +++ b/tools/snort2lua/preprocessor_states/pps_dcerpc_server.cc @@ -651,7 +651,7 @@ bool DcerpcServer::convert(std::istringstream& data_stream) bind["http_proxy"] = &bind_http_proxy; bind["http_server"] = &bind_http_server; - // FIXIT-N add when there is a way to make this play with http_inspect bindings + // FIXIT-M add when there is a way to make this play with http_inspect bindings // port 80 should not be added by default. If explicitly configured and conflicting // with other bindings, punt to wizard bind["http_proxy"]->print_binding(false);