Merge pull request #2276 in SNORT/snort3 from ~EBURMAI/snort3:appid_coverity_issues...

Squashed commit of the following:

commit 6de1af255f905a5d9ebd9789d6b161368593c16e
Author: Eduard Burmai <eburmai@cisco.com>
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 688caae7efea31facf4800455f3d2f03152fdbe1..514736bb8c24baa00255a099776b0547a45e3d7b 100644 (file)
--- 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 7c2a429edc1ce29ca318cd3d18ad63360b45453d..fceacba8957b1e329e8ba515d51e603fde06696e 100644 (file)
--- 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 93f7e600c438fb59509903d00dd14a1d4bbf84e1..62af8419dd2cd5facb006f9ed48c6cfa464bde8f 100644 (file)
--- 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<IpProtocol>(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<std::mutex> 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<IpProtocol>(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 44b67309141c9eea6cb4bbbf1d98318bb0e95822..08321e6f68cc14e6390c4c289c25569a9659236c 100644 (file)
--- 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 77dbc112b995f1c5efcb0f0646f7fed8450fc531..7b485a95f049b99711b702709d3874e06eb30983 100644 (file)
--- 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);
     }