From: Shravan Rangarajuvenkata (shrarang) Date: Mon, 29 Jun 2020 15:52:36 +0000 (+0000) Subject: Merge pull request #2276 in SNORT/snort3 from ~EBURMAI/snort3:appid_coverity_issues... X-Git-Tag: 3.0.2-1~12 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5af5850fb15323bc110b9ef27319269c4cbdd746;p=thirdparty%2Fsnort3.git Merge pull request #2276 in SNORT/snort3 from ~EBURMAI/snort3:appid_coverity_issues to master Squashed commit of the following: commit 6de1af255f905a5d9ebd9789d6b161368593c16e Author: Eduard Burmai Date: Thu Jun 18 06:28:35 2020 -0400 appid: Appid coverity issues --- diff --git a/src/network_inspectors/appid/app_forecast.cc b/src/network_inspectors/appid/app_forecast.cc index 688caae7e..514736bb8 100644 --- a/src/network_inspectors/appid/app_forecast.cc +++ b/src/network_inspectors/appid/app_forecast.cc @@ -78,7 +78,7 @@ void check_session_for_AF_indicator(Packet* p, AppidSessionDirection dir, AppId return; AFElement ind_element = af_indicator_entry->second; - AFActKey master_key(p, dir, ind_element.forecast, master_key); + AFActKey master_key(p, dir, ind_element.forecast); AFActVal new_active_value; new_active_value.target = ind_element.target; @@ -89,7 +89,7 @@ void check_session_for_AF_indicator(Packet* p, AppidSessionDirection dir, AppId AppId check_session_for_AF_forecast(AppIdSession& asd, Packet* p, AppidSessionDirection dir, AppId forecast) { - AFActKey master_key(p, dir, forecast, master_key); + AFActKey master_key(p, dir, forecast); //get out if there is no value auto check_act_val = AF_actives->find(master_key); diff --git a/src/network_inspectors/appid/app_forecast.h b/src/network_inspectors/appid/app_forecast.h index 7c2a429ed..fceacba89 100644 --- a/src/network_inspectors/appid/app_forecast.h +++ b/src/network_inspectors/appid/app_forecast.h @@ -55,12 +55,12 @@ PADDING_GUARD_BEGIN class AFActKey { public: - AFActKey(snort::Packet* p, AppidSessionDirection dir, AppId forecast, AFActKey &master_key) + AFActKey(snort::Packet* p, AppidSessionDirection dir, AppId forecast) : + forecast(forecast) { const snort::SfIp* src = dir ? p->ptrs.ip_api.get_dst() : p->ptrs.ip_api.get_src(); - memcpy(master_key.ip, src->get_ip6_ptr(), sizeof(master_key.ip)); - master_key.forecast = forecast; + memcpy(ip, src->get_ip6_ptr(), sizeof(ip)); } bool operator<(const AFActKey &key) const diff --git a/src/network_inspectors/appid/lua_detector_api.cc b/src/network_inspectors/appid/lua_detector_api.cc index 93f7e600c..62af8419d 100644 --- a/src/network_inspectors/appid/lua_detector_api.cc +++ b/src/network_inspectors/appid/lua_detector_api.cc @@ -130,6 +130,22 @@ static inline bool lua_params_validator(LuaDetectorParameters& ldp, bool packet_ return true; } +static inline int toipprotocol(lua_State *L, int index, + IpProtocol &proto, bool print_err = true) +{ + unsigned tmp_proto = lua_tointeger(L, index); + + if (tmp_proto > (unsigned)IpProtocol::RESERVED) + { + if (print_err) + ErrorMessage("Invalid protocol value %u\n", tmp_proto); + return -1; + } + + proto = static_cast(tmp_proto); + return 0; +} + int init(lua_State* L, int result) { lua_getglobal(L,"is_control"); @@ -202,13 +218,12 @@ static int service_register_pattern(lua_State* L) int index = 1; // FIXIT-M none of these params check for signedness casting issues - // FIXIT-M May want to create a lua_toipprotocol() so we can handle - // error checking in that function. - IpProtocol protocol = (IpProtocol)lua_tonumber(L, ++index); - if (protocol > IpProtocol::RESERVED) + + IpProtocol protocol; + if (toipprotocol(L, ++index, protocol)) { - ErrorMessage("Invalid protocol value %u\n", (unsigned)protocol); - return -1; + lua_pushnumber(L, -1); + return 1; } const char* pattern = lua_tostring(L, ++index); @@ -363,7 +378,13 @@ static int service_add_ports(lua_State* L) if (!init(L, 1)) return 1; ServiceDetectorPort pp; - pp.proto = (IpProtocol)lua_tonumber(L, 2); + + if (toipprotocol(L, 2, pp.proto)) + { + lua_pushnumber(L, -1); + return 1; + } + pp.port = lua_tonumber(L, 3); pp.reversed_validation = lua_tonumber(L, 5); @@ -846,7 +867,13 @@ static int client_register_pattern(lua_State* L) int index = 1; - IpProtocol protocol = (IpProtocol)lua_tonumber(L, ++index); + IpProtocol protocol; + if (toipprotocol(L, ++index, protocol)) + { + lua_pushnumber(L, -1); + return 1; + } + const char* pattern = lua_tostring(L, ++index); size_t size = lua_tonumber(L, ++index); unsigned int position = lua_tonumber(L, ++index); @@ -1127,14 +1154,11 @@ static int detector_add_host_port_application(lua_State* L) } unsigned port = lua_tointeger(L, ++index); - unsigned proto = lua_tointeger(L, ++index); - if (proto > (unsigned)IpProtocol::RESERVED) - { - ErrorMessage("%s:Invalid protocol value %u\n",__func__, proto); + IpProtocol proto; + if (toipprotocol(L, ++index, proto)) return 0; - } - if (!ud->get_odp_ctxt().host_port_cache_add(&ip_addr, (uint16_t)port, (IpProtocol)proto, type, app_id)) + if (!ud->get_odp_ctxt().host_port_cache_add(&ip_addr, (uint16_t)port, proto, type, app_id)) ErrorMessage("%s:Failed to backend call\n",__func__); return 0; @@ -1158,23 +1182,17 @@ static int detector_add_host_port_dynamic(lua_State* L) size_t ipaddr_size = 0; const char* ip_str = lua_tolstring(L, ++index, &ipaddr_size); if (!ip_str || !ipaddr_size || !convert_string_to_address(ip_str, &ip_addr)) - { - ErrorMessage("%s: Invalid IP address: %s\n",__func__, ip_str); return 0; - } unsigned port = lua_tointeger(L, ++index); - IpProtocol proto = (IpProtocol) lua_tointeger(L, ++index); - if (proto > IpProtocol::RESERVED) - { - ErrorMessage("%s:Invalid protocol value %u\n",__func__, (unsigned) proto); + IpProtocol proto; + if (toipprotocol(L, ++index, proto, false)) return 0; - } bool added = false; std::lock_guard lck(AppIdSession::inferred_svcs_lock); - if ( !host_cache[ip_addr]->add_service(port, proto, appid, true, &added) ) - ErrorMessage("%s:Failed to add host tracker service\n",__func__); + host_cache[ip_addr]->add_service(port, proto, appid, true, &added); + if (added) { AppIdSession::incr_inferred_svcs_ver(); @@ -1682,7 +1700,9 @@ static int detector_port_only_service(lua_State* L) AppId appId = lua_tointeger(L, ++index); uint16_t port = lua_tointeger(L, ++index); - IpProtocol protocol = static_cast(lua_tointeger(L, ++index)); + IpProtocol protocol; + if (toipprotocol(L, ++index, protocol)) + return 0; if (port == 0) ud->get_odp_ctxt().add_protocol_service_id(protocol, appId); @@ -1722,7 +1742,13 @@ static int detector_add_length_app_cache(lua_State* L) int index = 1; AppId appId = lua_tonumber(L, ++index); - IpProtocol proto = (IpProtocol)lua_tonumber(L, ++index); + IpProtocol proto; + if (toipprotocol(L, ++index, proto)) + { + lua_pushnumber(L, -1); + return 1; + } + uint8_t sequence_cnt = lua_tonumber(L, ++index); const char* sequence_str = lua_tostring(L, ++index); @@ -2256,7 +2282,10 @@ static int add_port_pattern_client(lua_State* L) size_t patternSize = 0; int index = 1; - IpProtocol protocol = (IpProtocol)lua_tonumber(L, ++index); + IpProtocol protocol; + if (toipprotocol(L, ++index, protocol)) + return 0; + 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); @@ -2307,7 +2336,10 @@ static int add_port_pattern_service(lua_State* L) size_t patternSize = 0; int index = 1; - IpProtocol protocol = (IpProtocol)lua_tonumber(L, ++index); + IpProtocol protocol; + if (toipprotocol(L, ++index, protocol)) + return 0; + uint16_t port = lua_tonumber(L, ++index); const char* pattern = lua_tolstring(L, ++index, &patternSize); unsigned position = lua_tonumber(L, ++index); @@ -2406,7 +2438,9 @@ static int create_future_flow(lua_State* L) return 0; uint16_t server_port = lua_tonumber(L, 5); - IpProtocol proto = (IpProtocol)lua_tonumber(L, 6); + IpProtocol proto; + if (toipprotocol(L, 6, proto)) + return 0; AppId service_id = lua_tointeger(L, 7); AppId client_id = lua_tointeger(L, 8); AppId payload_id = lua_tointeger(L, 9); @@ -2503,14 +2537,18 @@ static int get_http_tunneled_ip(lua_State* L) return 0; AppIdHttpSession* hsession = lsd->ldp.asd->get_http_session(); + if (hsession) + { + const TunnelDest* tunnel_dest = hsession->get_tun_dest(); - const TunnelDest* tunnel_dest = hsession->get_tun_dest(); - - if (!tunnel_dest) - lua_pushnumber(L, 0); - else - lua_pushnumber(L, tunnel_dest->ip.get_ip4_value()); + if (tunnel_dest) + { + lua_pushnumber(L, tunnel_dest->ip.get_ip4_value()); + return 1; + } + } + lua_pushnumber(L, 0); return 1; } @@ -2531,14 +2569,18 @@ static int get_http_tunneled_port(lua_State* L) return 0; AppIdHttpSession* hsession = lsd->ldp.asd->get_http_session(); + if (hsession) + { + const TunnelDest* tunnel_dest = hsession->get_tun_dest(); - const TunnelDest* tunnel_dest = hsession->get_tun_dest(); - - if (!tunnel_dest) - lua_pushnumber(L, 0); - else - lua_pushnumber(L, tunnel_dest->port); + if (tunnel_dest) + { + lua_pushnumber(L, tunnel_dest->port); + return 1; + } + } + lua_pushnumber(L, 0); return 1; } diff --git a/src/network_inspectors/appid/service_plugins/service_ftp.cc b/src/network_inspectors/appid/service_plugins/service_ftp.cc index 44b673091..08321e6f6 100644 --- a/src/network_inspectors/appid/service_plugins/service_ftp.cc +++ b/src/network_inspectors/appid/service_plugins/service_ftp.cc @@ -486,7 +486,10 @@ static int ftp_validate_reply(const uint8_t* data, uint16_t& offset, fd.rstate = FTP_REPLY_PARTIAL_EOL; else if (parse_ret == FTP_NOT_FOUND_EOL) { - check_ret_digit_code((data+temp), 3, 4 - (size - (temp)), fd.part_code_resp, fd ); + ret_code = check_ret_digit_code((data+temp), 3, 4 - (size - (temp)), fd.part_code_resp, fd ); + if(!ret_code) + return -1; + fd.rstate = FTP_REPLY_PARTIAL_RESP; fd.part_code_len = 3 - (size - (temp )); } diff --git a/src/network_inspectors/appid/tp_appid_utils.cc b/src/network_inspectors/appid/tp_appid_utils.cc index 77dbc112b..7b485a95f 100644 --- a/src/network_inspectors/appid/tp_appid_utils.cc +++ b/src/network_inspectors/appid/tp_appid_utils.cc @@ -430,8 +430,14 @@ static inline void process_ssl(AppIdSession& asd, if (asd.get_session_flags(APPID_SESSION_HTTP_TUNNEL)) { if (!asd.service_detector) - asd.service_detector = asd.ctxt.get_odp_ctxt().get_app_info_mgr(). - get_app_info_entry(APP_ID_SSL)->service_detector; + { + AppInfoTableEntry* entry = asd.ctxt.get_odp_ctxt(). + get_app_info_mgr().get_app_info_entry(APP_ID_SSL); + + if (entry) + asd.service_detector = entry->service_detector; + } + if (asd.get_session_flags(APPID_SESSION_HTTP_SESSION | APPID_SESSION_SPDY_SESSION)) asd.clear_session_flags(APPID_SESSION_HTTP_SESSION | APPID_SESSION_SPDY_SESSION); }