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

OK thanks. We will add those too.

> On Sep 2, 2015, at 1:09 PM, Bill Parker <[email protected]> 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
> [email protected]
> 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
[email protected]
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!