Lack of Sanity Checking for calls to strndup() in Snort-2.9.7.x/2.9.8 Beta

Hello All,

    In doing some code review, I found some instances where calls to
strndup() are not checked for a return value of NULL, indicating
failure (memory allocation is the same for strndup() as it is for
strdup(), btw).

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

In directory 'src/preprocessors/HttpInspect/server', file 'hi_server.c'
there is a call to strndup() at approximately line 675 which is missing
a test for a return value of NULL.  The patch file below should address
this issue:

--- hi_server.c.orig    2015-08-16 19:20:53.381000000 -0700
+++ hi_server.c 2015-08-16 19:28:01.482000000 -0700
@@ -673,6 +673,10 @@
         {
             headerLoc->len =  (end_ptr - cur_ptr);
             headerLoc->start = (u_char *)strndup((const char *)cur_ptr,
headerLoc->len);
+           if (headerLoc->start == NULL) /* oops, strndup failed   */
+           {   /* should we warn user that strndup() has failed?   */
+               return NULL;
+           }
         }
     }
     else

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

In directory 'src/dynamic-preprocessors/appid', file 'fw_appid.c', there
are instances of strndup() being called at lines: 892, 911, 917, 923,
933, and 943 which are not checked for a return value of NULL, which
would indicate failure.  The patch file below should address these
issues:

--- fw_appid.c.orig     2015-08-16 19:42:25.817000000 -0700
+++ fw_appid.c  2015-08-16 19:42:28.749000000 -0700
@@ -890,6 +890,11 @@
         {
             free(session->host);
             session->host = strndup((char *)headers->host.start,
headers->host.len);
+           if (session->host == NULL)
+           {
+               _dpd.errMsg("failed to allocate memory for session->host");
+               return;
+           }
             session->scan_flags |= SCAN_HTTP_HOST_URL_FLAG;

             if (headers->url.start)
@@ -909,18 +914,33 @@
         {
             free(session->useragent);
             session->useragent  = strndup((char
*)headers->userAgent.start, headers->userAgent.len);
+           if (session->useragent == NULL)
+           {
+               _dpd.errMsg("failed to allocate memory for session
user-agent");
+               return;
+           }
             session->scan_flags |= SCAN_HTTP_USER_AGENT_FLAG;
         }
         if (headers->referer.start)
         {
             free(session->referer);
             session->referer  = strndup((char *)headers->referer.start,
headers->referer.len);
+           if (session->referer == NULL)
+           {
+               _dpd.errMsg("failed to allocate memory for session
referer");
+               return;
+           }

         }
         if (headers->via.start)
         {
             free(session->via);
             session->via  = strndup((char *)headers->via.start,
headers->via.len);
+           if (session->via == NULL)
+           {
+               _dpd.errMsg("failed to allocate memory for session via");
+               return;
+           }
             session->scan_flags |= SCAN_HTTP_VIA_FLAG;
         }

@@ -931,6 +951,11 @@
         {
             free(session->via);
             session->via  = strndup((char *)headers->via.start,
headers->via.len);
+           if (session->via == NULL)
+           {
+               _dpd.errMsg("failed to allocate memory for session via");
+               return;
+           }
             session->scan_flags |= SCAN_HTTP_VIA_FLAG;
         }
         if (headers->responseCode.start)
@@ -941,6 +966,11 @@
             {
                 free(session->response_code);
                 session->response_code  = strndup((char
*)headers->responseCode.start, headers->responseCode.len);
+               if (session->response_code == NULL)
+               {
+                   _dpd.errMsg("failed to allocate memory for session
response code");
+                   return;
+               }
             }
         }
     }

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

I am attaching the patch files to this bug report.

Questions, comments, suggestions, complaints? :)

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!

--- hi_server.c.orig	2015-08-16 19:20:53.381000000 -0700
+++ hi_server.c	2015-08-16 19:28:01.482000000 -0700
@@ -673,6 +673,10 @@
         {
             headerLoc->len =  (end_ptr - cur_ptr);
             headerLoc->start = (u_char *)strndup((const char *)cur_ptr, headerLoc->len);
+	    if (headerLoc->start == NULL) /* oops, strndup failed   */
+	    {	/* should we warn user that strndup() has failed?   */
+		return NULL;
+	    }
         }
     }
     else

--- fw_appid.c.orig	2015-08-16 19:42:25.817000000 -0700
+++ fw_appid.c	2015-08-16 19:42:28.749000000 -0700
@@ -890,6 +890,11 @@
         {
             free(session->host);
             session->host = strndup((char *)headers->host.start, headers->host.len);
+	    if (session->host == NULL)
+	    {
+		_dpd.errMsg("failed to allocate memory for session->host");
+		return;
+	    }
             session->scan_flags |= SCAN_HTTP_HOST_URL_FLAG;
 
             if (headers->url.start)
@@ -909,18 +914,33 @@
         {
             free(session->useragent);
             session->useragent  = strndup((char *)headers->userAgent.start, headers->userAgent.len);
+	    if (session->useragent == NULL)
+	    {
+		_dpd.errMsg("failed to allocate memory for session user-agent");
+		return;
+	    }
             session->scan_flags |= SCAN_HTTP_USER_AGENT_FLAG;
         }
         if (headers->referer.start)
         {
             free(session->referer);
             session->referer  = strndup((char *)headers->referer.start, headers->referer.len);
+	    if (session->referer == NULL)
+	    {
+		_dpd.errMsg("failed to allocate memory for session referer");
+		return;
+	    }
 
         }
         if (headers->via.start)
         {
             free(session->via);
             session->via  = strndup((char *)headers->via.start, headers->via.len);
+	    if (session->via == NULL)
+	    {
+		_dpd.errMsg("failed to allocate memory for session via");
+		return;
+	    }
             session->scan_flags |= SCAN_HTTP_VIA_FLAG;
         }
 
@@ -931,6 +951,11 @@
         {
             free(session->via);
             session->via  = strndup((char *)headers->via.start, headers->via.len);
+	    if (session->via == NULL)
+	    {
+		_dpd.errMsg("failed to allocate memory for session via");
+		return;
+	    }
             session->scan_flags |= SCAN_HTTP_VIA_FLAG;
         }
         if (headers->responseCode.start)
@@ -941,6 +966,11 @@
             {
                 free(session->response_code);
                 session->response_code  = strndup((char *)headers->responseCode.start, headers->responseCode.len);
+		if (session->response_code == NULL)
+		{
+		    _dpd.errMsg("failed to allocate memory for session response code");
+		    return;
+		}
             }
         }
     }