Snort mailing list archives

Re: Missing sanity checks for calloc()/segment_calloc()/strdup() in Snort-2.9.8.0 beta


From: "Costas Kleopa (ckleopa)" <ckleopa () cisco com>
Date: Wed, 2 Sep 2015 17:18:20 +0000

OK thanks. We will add those too.

On Sep 2, 2015, at 1:09 PM, Bill Parker <wp02855 () gmail com> wrote:

Hello All,

    In reviewing source code in Snort 2.9.8.0, in directory
'snort-2.9.8.0_beta/src/preprocessors', file 'spp_sessions.c', there
are some calls to calloc() which are not checked for a return
value of NULL, indicating failure.  The patch file below should
correct/address these issues:

=======================================================================

--- spp_session.c.orig  2015-09-01 15:43:58.907000000 -0700
+++ spp_session.c       2015-09-01 15:49:41.865000000 -0700
@@ -27,6 +27,8 @@
  * @date    22 Feb 2013
  *
  * @brief  sessions? we don't got no sessions...
+ *
+ * @note   sessions? we don't need no steenking sessions...:)
  */
 
 /*  I N C L U D E S  ************************************************/
@@ -1012,6 +1014,10 @@
 
     session_configuration->numSnortPolicies = sc->num_policies_allocated;
     session_configuration->policy_ref_count = calloc( sc->num_policies_allocated, sizeof( uint32_t ) );
+    if (!session_configuration->policy_ref_count)
+    {
+       FatalError( "%s(%d) Could not allocate memory (calloc).\n", __FILE__, __LINE__ );
+    }
 
     return 0;
 }
@@ -3543,6 +3549,10 @@
 
     ssc->numSnortPolicies = sc->num_policies_allocated;
     ssc->policy_ref_count = calloc( sc->num_policies_allocated, sizeof( uint32_t ) );
+    if (!ssc->policy_ref_count)
+    {
+       FatalError( "%s(%d) Could not allocate memory (calloc).\n", __FILE__, __LINE__ );
+    }
 
     printSessionConfiguration(session_configuration);
 
 ======================================================================
 
 In directory 'snort-2.9.8.0_beta/src/dynamic-preprocessors/appid', file
 'commonAppMatcher.c', there are a pair of calls to calloc() which are
 not checked for a return value of NULL, indicating faliure.  The patch
 file below should address/correct these issues:
 
 --- commonAppMatcher.c.orig     2015-09-01 16:21:45.352000000 -0700
+++ commonAppMatcher.c  2015-09-01 16:25:56.476000000 -0700
@@ -983,6 +983,11 @@
 int AppIdCommonInit(tAppidStaticConfig *config)
 {
     pAppidActiveConfig = (tAppIdConfig *)calloc(1, sizeof(*pAppidActiveConfig));
+    if (!pAppidActiveConfig)
+    {
+       _dpd.errMsg("Config: Failed to allocate memory for Active Config in CommonInit()");
+       return -1;
+    }
 
     if (rnaFwConfigState == RNA_FW_CONFIG_STATE_UNINIT)
     {
@@ -1065,6 +1070,11 @@
 int AppIdCommonReload(void **new_context)
 {
     tAppIdConfig *pNewConfig = (tAppIdConfig *) calloc(1, sizeof(*pNewConfig));
+    if (!*pNewConfig)
+    {
+       _dpd.errMsg("Config: Failed to allocate memory for NewConfig in CommonReload()");
+       return -1;
+    }
     pAppidPassiveConfig = pNewConfig;
 
     // During a reload, C modules are not reloaded. Also, existing Lua modules are not reloaded.

=======================================================================     
 
In directory 'snort-2.9.8.0_beta/src/dynamic-preprocessors/appid'
file 'fw_appid.c', there are three calls to strdup() and a call to
calloc() which are not checked for a return value of NULL, indicating
error.  The patch file below should address/correct these issues:

--- fw_appid.c.orig     2015-09-01 16:35:46.955000000 -0700
+++ fw_appid.c  2015-09-01 16:42:36.125000000 -0700
@@ -2930,6 +2930,10 @@
                     appIdSession->hsession->chp_finished = 0;
                 }
                 appIdSession->hsession->host = strdup(attribute_data->spdyRequestHost);
+               if (!appIdSession->hsession->host)
+               {
+                   _dpd.errMsg("failed to allocate httpSession->host data");
+               }
                 appIdSession->scan_flags |= SCAN_HTTP_HOST_URL_FLAG;
             }
             if (attribute_data->spdyRequestPath)
@@ -2942,6 +2946,10 @@
                     appIdSession->hsession->chp_finished = 0;
                 }
                 appIdSession->hsession->uri = strdup(attribute_data->spdyRequestPath);
+               if (!addIdSession->hsession->uri)
+               {
+                   _dpd.errMsg("failed to allocate httpSession->uri data");
+               }
             }
             if (attribute_data->spdyRequestScheme &&
                 attribute_data->spdyRequestHost &&
@@ -2970,6 +2978,10 @@
                        strlen(attribute_data->spdyRequestHost) +
                        strlen(attribute_data->spdyRequestPath) + 4;
                 appIdSession->hsession->url = calloc(size, sizeof(char));
+               if (!appIdSession->hsession->url)
+               {
+                   DynamicPreprocessorFatalMessage("Could not allocate httpSession->url data");
+               }
                 snprintf(appIdSession->hsession->url, size, "%s://%s%s",
                          scheme, attribute_data->spdyRequestHost,
                          attribute_data->spdyRequestPath);
@@ -3408,6 +3420,11 @@
     if (flowp->username)
         free(flowp->username);
     flowp->username = strdup(username);
+    if (!flowp->username)
+    {
+       _dpd.errMsg("failed to allocate flowp->username in AddUser()");
+       return;
+    }
     flowp->usernameService = appId;
     if (success)
         setAppIdExtFlag(flowp, APPID_SESSION_LOGIN_SUCCEEDED);
         
=======================================================================

In directory 'snort-2.9.8.0_beta/src/dynamic-preprocessors/appid/service_plugins'
file 'service_MDNS.c', there is a call to calloc() which is not checked
for a return value of NULL, indicating failure.  The patch file below
should address/correct this issue:

--- service_MDNS.c.orig 2015-09-01 16:54:03.419000000 -0700
+++ service_MDNS.c      2015-09-01 16:54:56.721000000 -0700
@@ -459,6 +459,8 @@
 {
     unsigned i;
     tMdnsConfig *pMdnsConfig = calloc(1, sizeof(*pMdnsConfig));
+    if (!*pMdnsConfig)
+       return SERVICE_ENOMEM;
 
     if (!(pMdnsConfig->mdnsMatcher = _dpd.searchAPI->search_instance_new_ex(MPSE_ACF)))
     {

=======================================================================

In directory 'snort-2.9.8.0_beta/src/dynamic-preprocessors/reputation',
file 'reputation_config.c', there is a call to segment_calloc() which
is not checked for a return value of NULL, indicating failure.  The patch
file below should address/correct this issue:     

--- reputation_config.c.orig    2015-09-01 18:19:00.970000000 -0700
+++ reputation_config.c 2015-09-01 18:22:52.912000000 -0700
@@ -555,6 +555,10 @@
         }
 
         list_ptr = segment_calloc((size_t)DECISION_MAX, sizeof(ListInfo));
+       if (list_ptr == NULL)
+       {
+           DynamicPreprocessorFatalMessage("%s(%d): Failed to create IP list_ptr.\n", *(_dpd.config_file), 
*(_dpd.config_line));
+       }
         config->iplist->list_info = list_ptr;
 
         config->local_black_ptr = list_ptr + BLACKLISTED * sizeof(ListInfo);
         
=======================================================================

In directory 'snort-2.9.8.0_beta/src/dynamic-preprocessors/ssl_common',
file 'ssl_ha.c' there are two calls to calloc() which are not checked
for a return value of NULL, indicating failure.  The patch file below
should address/correct these issues:

--- ssl_ha.c.orig       2015-09-01 18:33:10.437000000 -0700
+++ ssl_ha.c    2015-09-01 18:38:06.162000000 -0700
@@ -184,6 +184,10 @@
         n_ssl_ha_funcs++;
 
     node = (SSLHAFuncsNode *)calloc(1, sizeof(SSLHAFuncsNode));
+    if (node == NULL)
+    {
+       DynamicPreprocessorFatalMessage("Unable to allocate memory for registration SSL HA types!\n");
+    }
     node->id = idx;
     node->mask = (1 << idx);
     node->preproc_id = (uint8_t) preproc_id;
@@ -408,6 +412,10 @@
     }
 
     pDefaultPolicyConfig->ssl_ha_config = (SSLHAConfig*)calloc(1, sizeof( SSLHAConfig ));
+    if (pDefaultPolicyConfig->ssl_ha_config == NULL)
+    {
+       DynamicPreprocessorFatalMessage("Unable to allocate memory for config SSL HA policy!\n");
+    }
 
     SSLParseHAArgs(sc, pDefaultPolicyConfig->ssl_ha_config, args);
 
=======================================================================

./configure --enable-sourcefire --enable-control-socket <enter>

finishes with no errors...:)

make (with above patches and previously submitted patches) finishes
cleanly with the output below:

make[3]: Leaving directory '/usr/local/src/snort-2.9.8.0_beta/tools/control'
make[3]: Entering directory '/usr/local/src/snort-2.9.8.0_beta/tools'
make[3]: Nothing to be done for 'all-am'.
make[3]: Leaving directory '/usr/local/src/snort-2.9.8.0_beta/tools'
make[2]: Leaving directory '/usr/local/src/snort-2.9.8.0_beta/tools'
make[2]: Entering directory '/usr/local/src/snort-2.9.8.0_beta'
make[2]: Leaving directory '/usr/local/src/snort-2.9.8.0_beta'
make[1]: Leaving directory '/usr/local/src/snort-2.9.8.0_beta'

 
I am attaching the patch files to this bug report...m00000!
 
Questions, Comments, Suggestions, Complaints? :)
 
Bill Parker (wp02855 at gmail dot com)
<spp_session.c.patch><commonAppMatcher.c.patch><fw_appid.c.patch><service_MDNS.c.patch><ssl_ha.c.patch>------------------------------------------------------------------------------
Monitor Your Dynamic Infrastructure at Any Scale With Datadog!
Get real-time metrics from all of your servers, apps and tools
in one place.
SourceForge users - Click here to start your Free Trial of Datadog now!
http://pubads.g.doubleclick.net/gampad/clk?id=241902991&iu=/4140_______________________________________________
Snort-devel mailing list
Snort-devel () lists sourceforge net
https://lists.sourceforge.net/lists/listinfo/snort-devel
Archive:
http://sourceforge.net/mailarchive/forum.php?forum_name=snort-devel

Please visit http://blog.snort.org for the latest news about Snort!


------------------------------------------------------------------------------
Monitor Your Dynamic Infrastructure at Any Scale With Datadog!
Get real-time metrics from all of your servers, apps and tools
in one place.
SourceForge users - Click here to start your Free Trial of Datadog now!
http://pubads.g.doubleclick.net/gampad/clk?id=241902991&iu=/4140
_______________________________________________
Snort-devel mailing list
Snort-devel () lists sourceforge net
https://lists.sourceforge.net/lists/listinfo/snort-devel
Archive:
http://sourceforge.net/mailarchive/forum.php?forum_name=snort-devel

Please visit http://blog.snort.org for the latest news about Snort!


Current thread: