Missing Sanity Checks in Snort-3.0.0 a2 (Build 163/165)
Bill Parker <[email protected]>
| Newsgroups | gmane.comp.security.ids.snort.devel |
|---|---|
| Message-ID | <CAFrbyQy6ZBwJWZwXYCOgWTscwSy9Uv6-xVrQn8j++mW5LHWtSQ@mail.gmail.com> |
Hello All,
In reviewing source code in Snort-3.0.0a2 (Build 163), I found
some instances where calls to calloc() and strdup() are
not checked for a return value of NULL, indicating failure.
=====================================================================
In directory 'snort-3.0.0-a2/src/test', file 'sfrt_test.cc',
a pair of calls to strdup() are not checked for a return
value of NULL, the patch file below should address these
issues:
--- sfrt_test.cc.orig 2015-08-13 18:54:01.343000000 -0700
+++ sfrt_test.cc 2015-08-13 18:58:42.309000000 -0700
@@ -104,6 +104,10 @@
sfip_pton(ip_entry->ip_str, &ip);
ip2_str = strdup(ip_entry->ip_str);
+ if (!ip2_str)
+ {
+ printf("Unable to duplicate ip_entry->ip_str\n");
+ }
p = strchr(ip2_str, '/');
if (p)
{
@@ -195,6 +199,10 @@
sfip_pton(ip_entry->ip_str, &ip);
ip2_str = strdup(ip_entry->ip_str);
+ if (!ip2_str)
+ {
+ printf("Unable to duplicate ip_entry->ip_str\n");
+ }
p = strchr(ip2_str, '/');
if (p)
{
=====================================================================
In directory 'snort-3.0.0-a2/tools/u2spewfoo', file 'u2spewfoo.cc',
there is a call to strdup() which is not checked for a return value
of NULL. The patch file below should address this issue:
--- u2spewfoo.cc.orig 2015-08-13 18:47:13.105000000 -0700
+++ u2spewfoo.cc 2015-08-13 18:49:52.056000000 -0700
@@ -67,6 +67,13 @@
ret->file = f;
ret->filename = strdup(filename);
+ if (!ret->filename)
+ {
+ printf("new_iterator: Call to strdup() failed.\n");
+ free(ret);
+ fclose(f);
+ return NULL;
+ }
return ret;
}
=====================================================================
In directory 'snort-3.0.0-a2/src/service_inspectors/ftp_telnet', file
'ftp.cc', there are two calls to calloc() which are not checked for
a return value of NULL, indicating failure. The patch file below
should address these issues:
--- ftp.cc.orig 2015-08-13 19:05:47.698000000 -0700
+++ ftp.cc 2015-08-13 19:08:41.172000000 -0700
@@ -316,6 +316,7 @@
Fmt = (FTP_PARAM_FMT*)calloc(1, sizeof(FTP_PARAM_FMT));
if (Fmt == NULL)
{
+ free(FTPCmd);
ParseAbort("Failed to allocate memory");
}
@@ -325,6 +326,8 @@
Fmt = (FTP_PARAM_FMT*)calloc(1, sizeof(FTP_PARAM_FMT));
if (Fmt == NULL)
{
+ free(FTPCmd->param_format);
+ free(FTPCmd);
ParseAbort("Failed to allocate memory");
}
=====================================================================
In directory 'snort-3.0.0-a2/src/network_inspectors/port_scan',
file 'ipobj.cc.patch', there is a call to strdup() which is not
checked for a return value of NULL, indicating failure. The patch
file below should address this issue:
Additionally, I had to bump the return values by 1 (since a -1
was already being returned, so this might need to be checked
from the calling routine as well)...
--- ipobj.cc.orig 2015-08-13 19:24:38.989000000 -0700
+++ ipobj.cc 2015-08-13 19:26:47.173000000 -0700
@@ -235,6 +235,10 @@
char* port2;
port_begin = strdup(portstr);
+ if (port_begin == NULL)
+ {
+ return -1; /* call to strdup() failed */
+ }
port1 = port_begin;
port2 = strstr(port_begin, "-");
@@ -243,7 +247,7 @@
if (*port1 == '\0')
{
free(port_begin);
- return -1;
+ return -2;
}
if (port2)
@@ -256,7 +260,7 @@
if (port_end == port1)
{
free(port_begin);
- return -2;
+ return -3;
}
if (port2)
@@ -265,7 +269,7 @@
if (port_end == port2)
{
free(port_begin);
- return -3;
+ return -4;
}
}
else
@@ -277,7 +281,7 @@
if ( port_hi > MAXPORTS-1 || port_lo > MAXPORTS-1)
{
free(port_begin);
- return -4;
+ return -5;
}
/* swap ports if necessary */
=====================================================================
I am attaching the patch files to this bug report...
Bill Parker (wp02855 at gmail dot com)
------------------------------------------------------------------------------
_______________________________________________
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!
snort3_ftp.cc.patch
(application/octet-stream, 582 B)
--- ftp.cc.orig 2015-08-13 19:05:47.698000000 -0700
+++ ftp.cc 2015-08-13 19:08:41.172000000 -0700
@@ -316,6 +316,7 @@
Fmt = (FTP_PARAM_FMT*)calloc(1, sizeof(FTP_PARAM_FMT));
if (Fmt == NULL)
{
+ free(FTPCmd);
ParseAbort("Failed to allocate memory");
}
@@ -325,6 +326,8 @@
Fmt = (FTP_PARAM_FMT*)calloc(1, sizeof(FTP_PARAM_FMT));
if (Fmt == NULL)
{
+ free(FTPCmd->param_format);
+ free(FTPCmd);
ParseAbort("Failed to allocate memory");
}
snort3_ipobj.cc.patch
(application/octet-stream, 1.1 KB)
--- ipobj.cc.orig 2015-08-13 19:24:38.989000000 -0700
+++ ipobj.cc 2015-08-13 19:26:47.173000000 -0700
@@ -235,6 +235,10 @@
char* port2;
port_begin = strdup(portstr);
+ if (port_begin == NULL)
+ {
+ return -1; /* call to strdup() failed */
+ }
port1 = port_begin;
port2 = strstr(port_begin, "-");
@@ -243,7 +247,7 @@
if (*port1 == '\0')
{
free(port_begin);
- return -1;
+ return -2;
}
if (port2)
@@ -256,7 +260,7 @@
if (port_end == port1)
{
free(port_begin);
- return -2;
+ return -3;
}
if (port2)
@@ -265,7 +269,7 @@
if (port_end == port2)
{
free(port_begin);
- return -3;
+ return -4;
}
}
else
@@ -277,7 +281,7 @@
if ( port_hi > MAXPORTS-1 || port_lo > MAXPORTS-1)
{
free(port_begin);
- return -4;
+ return -5;
}
/* swap ports if necessary */
snort3_sfrt.cc.patch
(application/octet-stream, 675 B)
--- sfrt_test.cc.orig 2015-08-13 18:54:01.343000000 -0700
+++ sfrt_test.cc 2015-08-13 18:58:42.309000000 -0700
@@ -104,6 +104,10 @@
sfip_pton(ip_entry->ip_str, &ip);
ip2_str = strdup(ip_entry->ip_str);
+ if (!ip2_str)
+ {
+ printf("Unable to duplicate ip_entry->ip_str\n");
+ }
p = strchr(ip2_str, '/');
if (p)
{
@@ -195,6 +199,10 @@
sfip_pton(ip_entry->ip_str, &ip);
ip2_str = strdup(ip_entry->ip_str);
+ if (!ip2_str)
+ {
+ printf("Unable to duplicate ip_entry->ip_str\n");
+ }
p = strchr(ip2_str, '/');
if (p)
{
snort3_u2spewfoo.cc.patch
(application/octet-stream, 347 B)
--- u2spewfoo.cc.orig 2015-08-13 18:47:13.105000000 -0700
+++ u2spewfoo.cc 2015-08-13 18:49:52.056000000 -0700
@@ -67,6 +67,13 @@
ret->file = f;
ret->filename = strdup(filename);
+ if (!ret->filename)
+ {
+ printf("new_iterator: Call to strdup() failed.\n");
+ free(ret);
+ fclose(f);
+ return NULL;
+ }
return ret;
}