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

Bill Parker wp02855 at ...2499...
Mon Aug 17 12:54:26 EDT 2015


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)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.snort.org/pipermail/snort-devel/attachments/20150817/dfcb2949/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: hi_server.c.patch
Type: application/octet-stream
Size: 463 bytes
Desc: not available
URL: <https://lists.snort.org/pipermail/snort-devel/attachments/20150817/dfcb2949/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fw_appid.c.patch
Type: application/octet-stream
Size: 2284 bytes
Desc: not available
URL: <https://lists.snort.org/pipermail/snort-devel/attachments/20150817/dfcb2949/attachment-0001.obj>


More information about the Snort-devel mailing list