From: Josh Date: Thu, 4 Sep 2014 16:15:57 +0000 (-0400) Subject: fixing some static analyzer bugs X-Git-Tag: 3.0.0-233~1417^2~6^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=945fbd0fc7aa407acaf63e20d84aeaf87b28b795;p=thirdparty%2Fsnort3.git fixing some static analyzer bugs --- 945fbd0fc7aa407acaf63e20d84aeaf87b28b795 diff --cc doc/CMakeLists.txt index 876d52198,876d52198..4e70de825 --- a/doc/CMakeLists.txt +++ b/doc/CMakeLists.txt @@@ -118,7 -118,7 +118,7 @@@ if (MAKE_PDF_DOC OR MAKE_HTML_DOC ${CMAKE_SOURCE_DIR}/tools/snort2lua > ${input} COMMAND ${CMAKE_COMMAND} -E copy ${input} ${output} -- DEPENDS snort2lua ++ DEPENDS snort2lua ${CMAKE_CURRENT_LIST_DIR}/get_differences.rb COMMENT "Documents: building config_changes.txt" ) list (APPEND ALL_SOURCES ${output}) diff --cc doc/config_changes.txt index 78df2e3da,78df2e3da..f41107482 --- a/doc/config_changes.txt +++ b/doc/config_changes.txt @@@ -17,30 -17,30 +17,42 @@@ change: ac-split ==> split_any_an change: ac-std ==> ac_std change: accumulate ==> reset = false change: acs ==> ac_sparse --change: addressspace_agnostic ==> address_space_agnostic change: alert_unified2 ==> unified2 change: alt_max_param_len ==> cmd_validity --change: autogenerate_preprocessor_decoder_rules ==> enable_builtin_rules change: bind_to ==> bindings change: bleedover-port-limit ==> bleedover_port_limit change: bleedover-warnings-enabled ==> bleedover_warnings_enabled change: block ==> base --change: checksum_mode ==> checksum_eval ++change: config addressspace_agnostic ==> packets.address_space_agnostic change: config alertfile: ==> alert_fast.file change: config alertfile: ==> alert_full.file ++change: config autogenerate_preprocessor_decoder_rules ==> ips.enable_builtin_rules ++change: config checksum_mode ==> network.checksum_eval ++change: config daq ==> daq.type ++change: config daq_dir ==> daq.dir ++change: config daq_mode ==> daq.mode ++change: config daq_var ==> daq.var ++change: config detection_filter ==> alerts.detection_filter_memcap ++change: config disable_inline_init_failopen ==> packets.enable_inline_init_failopen ++change: config enable_deep_teredo_inspection ==> udp.deep_teredo_inspection ++change: config event_filter ==> alerts.event_filter_memcap change: config file: file_block_timeout ==> block_timeout change: config file: file_lookup_timeout ==> lookup_timeout change: config file: file_signature_depth ==> signature_depth change: config file: file_type_depth ==> type_depth change: config file: signature ==> enable_signature change: config file: type_id ==> enable_type ++change: config max_attribute_hosts ==> attribute_table.max_hosts ++change: config max_attribute_services_per_host ==> attribute_table.max_services_per_host change: config mpls_payload_type: ethernet ==> mpls_payload_type = eth change: config mpls_payload_type: ipv4 ==> mpls_payload_type = ip4 change: config mpls_payload_type: ipv6 ==> mpls_payload_type = ip6 --change: daq ==> type --change: daq_dir ==> dir --change: daq_mode ==> mode --change: daq_var ==> var ++change: config nopcre ==> detection.pcre_enable ++change: config pkt_count ==> packets.limit ++change: config policy_mode ==> ips.mode ++change: config rate_filter ==> alerts.rate_filter_memcap ++change: config react ==> react.page ++change: config threshold ==> alerts.event_filter_memcap change: data_chan ==> ignore_data_chan change: debug-pkts ==> debug_pkts change: debug-print-fast-pattern ==> debug_print_fast_pattern @@@ -48,9 -48,9 +60,7 @@@ change: debug-print-nocontent-rule-tes change: debug-print-rule-group-build-details ==> debug_print_rule_group_build_details change: debug-print-rule-groups-compiled ==> debug_print_rule_groups_compiled change: debug-print-rule-groups-uncompiled ==> debug_print_rule_groups_uncompiled --change: detection_filter ==> detection_filter_memcap change: dgmlen ==> dgm_len --change: disable_inline_init_failopen ==> enable_inline_init_failopen change: disabled ==> enable change: dont_reassemble_async ==> reassemble_async change: dstport ==> dst_port @@@ -60,12 -60,12 +70,10 @@@ change: dynamicpreprocessor ==> plugin change: dynamicsidechannel ==> plugin_path change: enable-single-rule-group ==> enable_single_rule_group change: enable_cookie ==> enable_cookies --change: enable_deep_teredo_inspection ==> deep_teredo_inspection change: enabled ==> enable change: ethdst ==> eth_dst change: ethlen ==> eth_len change: ethsrc ==> eth_src --change: event_filter ==> event_filter_memcap change: fastpath-expensive-packets ==> fastpath_expensive_packets change: filename ==> file change: flow-file ==> flow_file = true @@@ -115,8 -115,8 +123,6 @@@ change: max-pattern-len ==> max_patter change: max-pkt-time ==> max_pkt_time change: max-rule-time ==> max_rule_time change: max_active_responses ==> max_responses --change: max_attribute_hosts ==> max_hosts --change: max_attribute_services_per_host ==> max_services_per_host change: max_icmp ==> max_sessions change: max_ip ==> max_sessions change: max_queued_bytes ==> queue_limit.max_bytes @@@ -126,27 -126,27 +132,22 @@@ change: max_udp ==> max_session change: min_fragment_length ==> min_frag_length change: min_response_seconds ==> min_interval change: non_rfc_char ==> non_rfc_chars --change: nopcre ==> pcre_enable change: overlap_limit ==> max_overlaps change: pad ==> base change: paf_max [0:63780] ==> paf_max [1460:63780] change: pkt-log ==> pkt_log --change: pkt_count ==> limit change: pktcnt ==> packets change: policy bsd-right ==> policy = bsd_right --change: policy_mode ==> mode change: ports ==> bindings change: ports ==> gtp_ports change: post_depth [-1:65495] ==> post_depth [-1:65535] change: preprocessor frag3_engine: timeout 0 ==> session_timeout 256 --change: preprocessor normalize_icmp4 ==> icmp4 == --change: preprocessor normalize_icmp6 ==> icmp6 == --change: preprocessor normalize_ip6 ==> ip6 == ++change: preprocessor normalize_icmp4 ==> normalize.icmp4 ++change: preprocessor normalize_icmp6 ==> normalize.icmp6 ++change: preprocessor normalize_ip6 ==> normalize.ip6 change: print ==> count change: proto ==> protos change: prune_log_max ==> histogram --change: rate_filter ==> rate_filter_memcap --change: react ==> page change: req_pay ==> base change: req_urg ==> base change: req_urp ==> base @@@ -182,7 -182,7 +183,6 @@@ change: tcpflags ==> tcp_flag change: tcplen ==> tcp_len change: tcpseq ==> tcp_seq change: tcpwindow ==> tcp_win --change: threshold ==> event_filter_memcap change: time ==> seconds change: timeout ==> session_timeout change: trim_mss ==> trim diff --cc doc/get_differences.rb index 27fc8b6df,27fc8b6df..7e3511524 --- a/doc/get_differences.rb +++ b/doc/get_differences.rb @@@ -3,7 -3,7 +3,7 @@@ # CONST REG_EX. DO NOT CHANGE delete_pattern = /add_deleted_comment\(\"(.*)\"\);/ diff_pattern = /add_diff_option_comment\(\"(.*)\",\s?\"(.*)\"\)/ --template_diff = /<\s*&(.*),.*,\s*&(.*)>/ ++template_diff = /<\s*&(.*),\s*&(.*),\s*&(.*)>/ config_delete_template = /deleted_ctor<&(.*)>/ paths_diff = /paths_ctor<\s*&(.*)\s*>/ # check kws_paths.cc normalizers_diff = /norm_sans_options_ctor<\s?&(.*)>/ # check pps_normalizers @@@ -36,11 -36,11 +36,10 @@@ Dir.glob("#{dir}/**/*cc").each do |file if line =~ diff_pattern arr << "change: #{$1} ==> #{$2}" -- end if line =~ template_diff -- arr << "change: #{$1} ==> #{$2}" ++ arr << "change: config #{$1} ==> #{$2}.#{$3}" end if line =~ config_delete_template @@@ -54,7 -54,7 +53,7 @@@ end if line =~ normalizers_diff -- arr << "change: preprocessor normalize_#{$1} ==> #{$1} == " ++ arr << "change: preprocessor normalize_#{$1} ==> normalize.#{$1}" end if line =~ unified2_diff diff --cc src/codecs/ip/checksum.h index c8f386436,c8f386436..03580683e --- a/src/codecs/ip/checksum.h +++ b/src/codecs/ip/checksum.h @@@ -36,7 -36,7 +36,8 @@@ namespace checksu struct Pseudoheader6 { -- uint32_t sip[4], dip[4]; ++ uint32_t sip[4]; ++ uint32_t dip[4]; uint8_t zero; uint8_t protocol; uint16_t len; @@@ -45,7 -45,7 +46,9 @@@ struct Pseudoheader { -- uint32_t sip, dip; ++ ++ uint32_t sip; ++ uint32_t dip; uint8_t zero; uint8_t protocol; uint16_t len; @@@ -78,6 -78,6 +81,25 @@@ inline uint16_t ip_cksum(const uint16_ namespace detail { ++ ++struct PsuedoheaderUnion ++{ ++ union ++ { ++ Pseudoheader ph4; ++ uint16_t ph4_arr[12]; ++ }; ++}; ++ ++struct Psuedoheader6Union ++{ ++ union ++ { ++ Pseudoheader ph6; ++ uint16_t ph6_arr[18]; ++ }; ++}; ++ static inline uint16_t cksum_add(const uint16_t *buf, std::size_t len, uint32_t cksum) { const uint16_t *sp = buf; @@@ -158,9 -158,9 +180,17 @@@ } --static inline void add_ipv4_pseudoheader(const uint16_t* const h, ++static inline void add_ipv4_pseudoheader(const Pseudoheader* const ph4, uint32_t &cksum) { ++ /* ++ * This mess is necessary to make static analyzers happy. ++ * Otherwise they assume we are reading garbage values ++ */ ++ const PsuedoheaderUnion* const ph4_u = reinterpret_cast ++ (ph4); ++ const uint16_t* const h = ph4_u->ph4_arr; ++ /* ipv4 pseudo header must have 12 bytes */ cksum += h[0]; cksum += h[1]; @@@ -171,28 -171,28 +201,36 @@@ } --static inline void add_ipv6_pseudoheader(const uint16_t* const h, ++static inline void add_ipv6_pseudoheader(const Pseudoheader6* const ph6, uint32_t &cksum) { -- /* PseudoHeader must have 36 bytes */ -- cksum += h[0]; -- cksum += h[1]; -- cksum += h[2]; -- cksum += h[3]; -- cksum += h[4]; -- cksum += h[5]; -- cksum += h[6]; -- cksum += h[7]; -- cksum += h[8]; -- cksum += h[9]; -- cksum += h[10]; -- cksum += h[11]; -- cksum += h[12]; -- cksum += h[13]; -- cksum += h[14]; -- cksum += h[15]; -- cksum += h[16]; -- cksum += h[17]; ++ /* ++ * This mess is necessary to make static analyzers happy. ++ * Otherwise they assume we are reading garbage values ++ */ ++ const Psuedoheader6Union* const ph6_u = reinterpret_cast ++ (ph6); ++ const uint16_t* const h = ph6_u->ph6_arr; ++ ++ /* PseudoHeader must have 36 bytes */ ++ cksum += h[0]; ++ cksum += h[1]; ++ cksum += h[2]; ++ cksum += h[3]; ++ cksum += h[4]; ++ cksum += h[5]; ++ cksum += h[6]; ++ cksum += h[7]; ++ cksum += h[8]; ++ cksum += h[9]; ++ cksum += h[10]; ++ cksum += h[11]; ++ cksum += h[12]; ++ cksum += h[13]; ++ cksum += h[14]; ++ cksum += h[15]; ++ cksum += h[16]; ++ cksum += h[17]; } @@@ -256,7 -256,7 +294,7 @@@ inline uint16_t icmp_cksum(const uint16 { uint32_t cksum = 0; -- detail::add_ipv6_pseudoheader((const uint16_t* const)ph, cksum); ++ detail::add_ipv6_pseudoheader(ph, cksum); return detail::cksum_add(buf, len, cksum); } @@@ -272,7 -272,7 +310,7 @@@ inline uint16_t tcp_cksum(const uint16_ { uint32_t cksum = 0; -- detail::add_ipv4_pseudoheader((const uint16_t* const)ph, cksum); ++ detail::add_ipv4_pseudoheader(ph, cksum); detail::add_tcp_header(h, len, cksum); return detail::cksum_add(h, len, cksum); } @@@ -284,7 -284,7 +322,7 @@@ inline uint16_t tcp_cksum(const uint16_ { uint32_t cksum = 0; -- detail::add_ipv6_pseudoheader((const uint16_t* const)ph, cksum); ++ detail::add_ipv6_pseudoheader(ph, cksum); detail::add_tcp_header(buf, len, cksum); return detail::cksum_add(buf, len, cksum); } @@@ -296,7 -296,7 +334,7 @@@ inline uint16_t udp_cksum(const uint16_ { uint32_t cksum = 0; -- detail::add_ipv4_pseudoheader((const uint16_t* const)ph, cksum); ++ detail::add_ipv4_pseudoheader(ph, cksum); detail::add_udp_header(buf, len, cksum); return detail::cksum_add(buf, len, cksum); } @@@ -308,7 -308,7 +346,7 @@@ inline uint16_t udp_cksum(const uint16_ { uint32_t cksum = 0; -- detail::add_ipv6_pseudoheader((const uint16_t* const)ph, cksum); ++ detail::add_ipv6_pseudoheader(ph, cksum); detail::add_udp_header(buf, len, cksum); return detail::cksum_add(buf, len, cksum); } diff --cc src/codecs/misc/Makefile.am index b61583e9d,b61583e9d..a3fe55a7c --- a/src/codecs/misc/Makefile.am +++ b/src/codecs/misc/Makefile.am @@@ -32,7 -32,7 +32,7 @@@ libcd_icmp6_ip_la_SOURCES = cd_icmp6_ip ehlib_LTLIBRARIES += libcd_llc.la libcd_llc_la_CXXFLAGS = $(AM_CXXFLAGS) -DBUILDING_SO libcd_llc_la_LDFLAGS = -export-dynamic -shared --libcd_llc_la_SOURCES = libcd_llc.cc ++libcd_llc_la_SOURCES = cd_llc.cc ehlib_LTLIBRARIES += libcd_gtp.la libcd_gtp_la_CXXFLAGS = $(AM_CXXFLAGS) -DBUILDING_SO diff --cc src/codecs/misc/cd_icmp4_ip.cc index 1f4b93e16,1f4b93e16..e769b4bd9 --- a/src/codecs/misc/cd_icmp4_ip.cc +++ b/src/codecs/misc/cd_icmp4_ip.cc @@@ -89,7 -89,7 +89,6 @@@ bool Icmp4IpCodec::decode(const uint8_ return false; } -- ip_len = ntohs(ip4h->get_len());/* set the IP datagram length */ hlen = ip4h->get_hlen() << 2; /* set the IP header length */ if(raw_len < hlen) diff --cc src/file_api/libs/file_config.cc index d9c9e5eda,d9c9e5eda..670daa865 --- a/src/file_api/libs/file_config.cc +++ b/src/file_api/libs/file_config.cc @@@ -314,6 -314,6 +314,7 @@@ static uint8_t* convertTextToHex(char * ParseError("content hexmode argument has invalid " "number of hex digits. The argument '%s' " "must contain a full even byte string.", current_ptr); ++ free(hex); return (uint8_t*)""; } @@@ -324,7 -324,7 +325,8 @@@ else { ParseError("'%c' is not a valid hex value, please input hex values (0x0 - 0xF)", -- (char) *current_ptr); ++ (char) *current_ptr); ++ free(hex); return (uint8_t*)""; } @@@ -337,7 -337,7 +339,8 @@@ else { ParseError("'%c' is not a valid hex value, please input hex values (0x0 - 0xF)", -- (char) *current_ptr); ++ (char) *current_ptr); ++ free(hex); return (uint8_t*)""; } DEBUG_WRAP(DebugMessage(DEBUG_FILE,"Hex buffer: %s\n", hex_buf);); @@@ -604,6 -604,6 +607,7 @@@ void parse_file_rule(const char *args, if (file_config->FileRules[rule->id]) { ParseError("file type: duplicated rule id %d defined!", rule->id); ++ free(rule); return; } file_config->FileRules[rule->id] = rule; diff --cc src/log/log_text.cc index c500374ae,c500374ae..3e04c1589 --- a/src/log/log_text.cc +++ b/src/log/log_text.cc @@@ -888,7 -888,7 +888,7 @@@ void LogTcpOptions(TextLog* log, cons } if (j < opts_len) -- TextLog_Print(log, "%02x", opts[i].data[j]); ++ TextLog_Print(log, "%02x", 0); } TextLog_Putc(log, ' '); diff --cc src/loggers/Makefile.am index d3ac06cf7,d3ac06cf7..b3b1d1bb5 --- a/src/loggers/Makefile.am +++ b/src/loggers/Makefile.am @@@ -3,6 -3,6 +3,7 @@@ AUTOMAKE_OPTIONS=foreign no-dependencie noinst_LIBRARIES = libloggers.a libloggers_a_SOURCES = \ alert_luajit.cc \ ++log_codecs.cc \ loggers.cc \ loggers.h diff --cc src/loggers/log_codecs.cc index 3ba609c3b,3ba609c3b..6fdb167b7 --- a/src/loggers/log_codecs.cc +++ b/src/loggers/log_codecs.cc @@@ -193,10 -193,10 +193,10 @@@ static const LogApi log_codecs_api mod_ctor, mod_dtor }, -- (OUTPUT_TYPE_FLAG__LOG | OUTPUT_TYPE_FLAG__ALERT), ++ OUTPUT_TYPE_FLAG__LOG, codec_log_ctor, codec_log_dtor }; --const BaseApi* eh_codecs = &log_codecs_api.base; ++const BaseApi* log_codecs = &log_codecs_api.base; diff --cc src/loggers/loggers.cc index ad0c37283,ad0c37283..dda8a989a --- a/src/loggers/loggers.cc +++ b/src/loggers/loggers.cc @@@ -27,7 -27,7 +27,7 @@@ #include "framework/logger.h" // to ensure PacketManager::log_protocols() is built into Snort++ --extern const BaseApi* eh_codecs; ++extern const BaseApi* log_codecs; extern const BaseApi* log_luajit; #ifdef LINUX @@@ -68,8 -68,8 +68,8 @@@ const BaseApi* loggers[] eh_unified2, #endif // loggers ++ log_codecs, log_luajit, // both -- eh_codecs, nullptr }; diff --cc src/parser/parse_stream.cc index 84c106c5e,84c106c5e..6fb748d7a --- a/src/parser/parse_stream.cc +++ b/src/parser/parse_stream.cc @@@ -175,9 -175,9 +175,7 @@@ static TokenType get_token state = 3; break; case 5: // unquoted escape -- if ( c == '\n' ) -- state = 0; -- else if ( c != '\r' ) ++ if ( c != '\n' && c != '\r' ) printf("error: invalid escape on line %d\n", lines); state = 0; break; diff --cc src/service_inspectors/ftp_telnet/hi_util_kmap.cc index 6fd7e91ea,6fd7e91ea..d994557df --- a/src/service_inspectors/ftp_telnet/hi_util_kmap.cc +++ b/src/service_inspectors/ftp_telnet/hi_util_kmap.cc @@@ -47,6 -47,6 +47,8 @@@ #include "config.h" #endif ++#include ++#include #include #include #include @@@ -55,9 -55,9 +57,8 @@@ #include "hi_util_xmalloc.h" //#define MEMASSERT(p) if(!p){printf("KMAP-No Memory: File: %s Line:%d!\n",__FILE__,__LINE__);exit(0);} -- #define MEMASSERT(p) --#define LOWERCASE tolower ++ /* * @@@ -238,24 -238,24 +239,29 @@@ static KMAPNODE * KMapCreateNode(KMAP */ int KMapAdd( KMAP *km, void * key, int n, void * userdata ) { -- int i,ksize; -- int type = 0; -- unsigned char *P = (unsigned char *)key; -- KMAPNODE *root; -- unsigned char xkey[256]; ++ int i,ksize; ++ int type = 0; ++ const unsigned char *P = (unsigned char *)key; ++ KMAPNODE *root; ++ std::string xkey; if( n <= 0 ) { -- n = strlen( (char*) key ); -- if( n > (int)sizeof(xkey) ) ++ std::size_t tmp_len = strlen( (char*) key); ++ if (tmp_len > std::numeric_limits::max()) return -99; ++ ++ n = tmp_len; } if( km->nocase ) { ++ xkey.resize(n); ++ for(i=0;i (int)sizeof(xkey) ) -- return 0; ++ std::size_t tmp_len = strlen( (char*) key); ++ if (tmp_len > std::numeric_limits::max()) ++ return nullptr; ++ n = tmp_len; } ++ if( ks->nocase ) { ++ xkey.resize(n); for(i=0;i ++#include #include #include #include @@@ -55,9 -55,9 +57,7 @@@ #include "hi_util_xmalloc.h" //#define MEMASSERT(p) if(!p){printf("KMAP-No Memory: File: %s Line:%d!\n",__FILE__,__LINE__);exit(0);} -- #define MEMASSERT(p) --#define LOWERCASE tolower /* * @@@ -241,22 -241,22 +241,27 @@@ int KMapAdd( KMAP *km, void * key, int { int i,ksize; int type = 0; -- unsigned char *P = (unsigned char *)key; ++ const unsigned char *P = (unsigned char *)key; KMAPNODE *root; -- unsigned char xkey[256]; ++ std::string xkey; if( n <= 0 ) { -- n = strlen( (char*) key ); -- if( n > (int)sizeof(xkey) ) ++ const std::size_t tmp_len = strlen( (char*) key); ++ if (tmp_len > std::numeric_limits::max()) return -99; ++ ++ n = (int) tmp_len; } if( km->nocase ) { ++ xkey.resize(n); ++ for(i=0;i (int)sizeof(xkey) ) -- return 0; ++ const std::size_t tmp_len = strlen( (char*) key); ++ if (tmp_len > std::numeric_limits::max()) ++ return nullptr; ++ n = (int) tmp_len; } ++ if( ks->nocase ) { ++ xkey.resize(n); for(i=0;i