[Snort-devel] Missing Sanity Checks in Snort-3.0.0 a2 (Build 163/165)

Bill Parker wp02855 at ...2499...
Fri Aug 14 12:29:37 EDT 2015


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)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.snort.org/pipermail/snort-devel/attachments/20150814/034c0097/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: snort3_ftp.cc.patch
Type: application/octet-stream
Size: 582 bytes
Desc: not available
URL: <https://lists.snort.org/pipermail/snort-devel/attachments/20150814/034c0097/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: snort3_ipobj.cc.patch
Type: application/octet-stream
Size: 1102 bytes
Desc: not available
URL: <https://lists.snort.org/pipermail/snort-devel/attachments/20150814/034c0097/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: snort3_sfrt.cc.patch
Type: application/octet-stream
Size: 675 bytes
Desc: not available
URL: <https://lists.snort.org/pipermail/snort-devel/attachments/20150814/034c0097/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: snort3_u2spewfoo.cc.patch
Type: application/octet-stream
Size: 347 bytes
Desc: not available
URL: <https://lists.snort.org/pipermail/snort-devel/attachments/20150814/034c0097/attachment-0003.obj>


More information about the Snort-devel mailing list