Snort mailing list archives
Lack of sanity checks and possible memory leak in 2.9.7.x
From: Bill Parker <wp02855 () gmail com>
Date: Mon, 13 Apr 2015 10:04:56 -0700
Hello All, In reviewing code in directory 'src/detection-plugins', file file 'sp_byte_jump.c', I found a pair of calls to strdup() which are not tested for a return value of NULL, indicating failure. The patch file below corrects this issue: --- sp_byte_jump.c.orig 2015-04-11 16:00:38.317385324 -0700 +++ sp_byte_jump.c 2015-04-11 16:03:23.829421053 -0700 @@ -201,7 +201,13 @@ ByteJumpOverrideData *new = SnortAlloc(sizeof(ByteJumpOverrideData)); new->keyword = strdup(keyword); + if (new->keyword == NULL) /* should we throw fatal error here or just return? */ + return; new->option = strdup(option); + if (new->option == NULL) { /* should we throw fatal error here or just return? */ + free(new->keyword); + return; + } new->func = roo_func; new->next = byteJumpOverrideFuncs; In directory 'src/dynamic-plugins', file 'sf_convert_dynamic.c', I found a call to 'strdup()' which is not checked for a return value of NULL, indicating failure. The patch file below corrects this issue: --- sf_convert_dynamic.c.orig 2015-04-11 16:17:17.987438247 -0700 +++ sf_convert_dynamic.c 2015-04-11 16:17:25.803431620 -0700 @@ -877,6 +877,9 @@ snort_byte->offset = so_byte->offset; snort_byte->align = so_byte->align; snort_byte->name = strdup(so_byte->refId); + if (snort_byte->name == NULL) { /* couldn't allocate memory, now what? */ + return -1; /* what should be returned here? */ + } /* In an SO rule, setting multiplier to 0 means that the multiplier is ignored. This is not the case in the text rule version of byte_extract. */ In directory 'src/dynamic-preprocessors/appid/service_plugins', file 'service_rpc.c', there is a call to strdup(), which upon failure, does not free the previously allocated memory assigned to variable 'prog' above it. The patch file below corrects this issue: --- service_rpc.c.orig 2015-04-11 16:55:21.199622775 -0700 +++ service_rpc.c 2015-04-11 16:56:47.197552670 -0700 @@ -241,8 +241,10 @@ prog->next = rpc_programs; rpc_programs = prog; prog->name = strdup(rpc->r_name); - if (!prog->name) + if (!prog->name) { + free(prog); /* free previously alloc'd memory? */ _dpd.errMsg("failed to allocate rpc program name"); + } } } } In directory 'src/dynamic-preprocessors/appid/service_plugins', file 'service_tftp.c', I found an instance where a call to calloc() is made for variable 'tmp_td', but upon failure does not release the memory allocated to variable 'td' in function 'MakeRNAServiceValidationPrototype(tftp_validate)', should this memory not be released if return SERVICE_ENOMEM is called from the function? Patch file is below: --- service_tftp.c.orig 2015-04-11 17:08:32.411181696 -0700 +++ service_tftp.c 2015-04-11 17:11:32.209033078 -0700 @@ -194,7 +194,11 @@ goto bail; tmp_td = calloc(1, sizeof(ServiceTFTPData)); - if (tmp_td == NULL) return SERVICE_ENOMEM; + if (tmp_td == NULL) + { + free(td); + return SERVICE_ENOMEM; + } tmp_td->state = TFTP_STATE_TRANSFER; dip = GET_DST_IP(pkt); In directory 'src/dynamic-preprocessors/ftptelnet', file 'snort_ftptelnet.c' there is a call to strdup() which is not checked for NULL, indicating failure. The patch file is below: --- snort_ftptelnet.c.orig 2015-04-11 17:27:55.717031887 -0700 +++ snort_ftptelnet.c 2015-04-11 17:29:01.636409901 -0700 @@ -3394,6 +3394,11 @@ ftp_conf = GlobalConf->default_ftp_server; ConfigParseResumePtr = server+strlen(server); GlobalConf->default_ftp_server->serverAddr = strdup("default"); + if (GlobalConf->default_ftp_server->serverAddr == NULL) + { + DynamicPreprocessorFatalMessage("Out of memory trying to create " + "default ftp server configuration.\n"); + } iRet = parseFtpServerConfigStr( ftp_conf, ConfigParseResumePtr, ip_list, ErrorString, ErrStrLen ); if (iRet) { In directory 'src/dynamic-preprocessors/ssl_common', file 'ssl_ha.c', there are some instances of calls to calloc() without a check for a return value of NULL, indicating failure. The patch file which corrects this issue is below: --- ssl_ha.c.orig 2015-04-11 18:17:32.320342862 -0700 +++ ssl_ha.c 2015-04-11 18:21:07.955749013 -0700 @@ -184,6 +184,10 @@ n_ssl_ha_funcs++; node = (SSLHAFuncsNode *)calloc(1, sizeof(SSLHAFuncsNode)); + if (node == NULL) + { + DynamicPreprocessorFatalMessage("Failed to allocate storage for SSL HA functions.\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("Failed to allocate storage for SSL HA configuration.\n"); + } SSLParseHAArgs(sc, pDefaultPolicyConfig->ssl_ha_config, args); in directory 'src', file 'log.c', it would appear that the call to calloc() is commented out, which would make the following code appear to do nothing (unless data_dump_buffer is declared and initialized elsewhere, which does not appear to be the case): /* allocate a buffer to print the data to */ //data_dump_buffer = (char *) calloc(data_len + (data_len >> 6) + 2, sizeof(char)); if (data_dump_buffer == NULL) { AllocDumpBuf(); } I am attaching the patch file(s) to this bug report... Bill Parker (wp02855 at gmail dot com)
Attachment:
ssl_ha.c.patch
Description:
Attachment:
service_rpc.c.patch
Description:
Attachment:
service_tftp.c.patch
Description:
Attachment:
sf_convert_dynamic.c.patch
Description:
Attachment:
snort_ftptelnet.c.patch
Description:
Attachment:
sp_byte_jump.c.patch
Description:
------------------------------------------------------------------------------ BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT Develop your own process in accordance with the BPMN 2 standard Learn Process modeling best practices with Bonita BPM through live exercises http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_ source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
_______________________________________________ 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:
- Lack of sanity checks and possible memory leak in 2.9.7.x Bill Parker (Apr 13)