[Snort-devel] [snort-devel] Patches to add error checking and replace legacy library calls in 2.9.7.0-alpha

Costas Kleopa (ckleopa) ckleopa at ...3461...
Wed Mar 12 13:41:34 EDT 2014


Bill.

Thanks for the updates on this. We will bug this too.

Thanks,
Costas.

From: Bill Parker <wp02855 at ...2499...<mailto:wp02855 at ...2499...>>
Date: Wednesday, March 12, 2014 at 12:32 PM
To: "snort-devel at lists.sourceforge.net<mailto:snort-devel at ...362....net>" <snort-devel at lists.sourceforge.net<mailto:snort-devel at ...2763...rge.net>>
Subject: [Snort-devel] [snort-devel] Patches to add error checking and replace legacy library calls in 2.9.7.0-alpha

Hi All,

   In reviewing code in Snort-2.9.7.0-alpha, I found instances
of the legacy library call 'index()' which I have replaced with
the more up to date library call 'strchr()' in directory
/snort-2.9.7.0-alpha/tools/u2streamer/', file 'SpoolFileIterator.c'

I also found a call to stat() and calls to write() which did not
check for a return value of < 0, indicating failure (according to
XXX comments in this file, it says we may want to check for errors)
so this patch does that.

The patch file is below:

--- SpoolFileIterator.c.orig    2014-03-11 18:34:12.895026389 -0700
+++ SpoolFileIterator.c 2014-03-11 19:25:49.640606828 -0700
@@ -175,11 +175,11 @@
     }

     /* Remove trailing newline */
-    if((s_position = index(buffer, '\n')))
+    if((s_position = strchr(buffer, '\n')))
         *s_position = '\0';

     /* Parse the position */
-    if(!(s_position = index(buffer, ',')))
+    if(!(s_position = strchr(buffer, ',')))
     {
         fprintf(stderr, "Syntax error processing bookmark data '%s'\n",
                 buffer);
@@ -228,6 +228,7 @@
     char buffer[256];
     if(!iterator || !iterator->bookmark_file)
         return SF_EINVAL;
+    int offset;

     if(iterator->bookmark_fd == -1)
     {
@@ -245,8 +246,22 @@
     memset(buffer, 256, ' ');
     snprintf(buffer, 255, "%u, %u\n", timestamp, position);
     /* Set back to the beginning of the file */
-    lseek(iterator->bookmark_fd, 0, SEEK_SET);
-    write(iterator->bookmark_fd, buffer, 256);
+    offset = 0;
+    offset = lseek(iterator->bookmark_fd, 0, SEEK_SET);
+    if (offset == -1)
+    {
+       fprintf(stderr, "Unable to seek file '%s': %s\n",
+               iterator->bookmark_file, strerror(errno));
+       return -1;
+    }
+    offset = 0;
+    offset = write(iterator->bookmark_fd, buffer, 256);
+    if (offset == -1)
+    {
+       fprintf(stderr, "Unable to write file '%s': %s\n",
+               iterator->bookmark_file, strerror(errno));
+       return -1;
+    }

     /* XXX Block signals here */
     /* XXX We may also want to check for errors */
@@ -292,7 +307,12 @@
                     return SF_ENOENT;
                 }

-                stat(iterator->filepath, &buf);
+                rval = stat(iterator->filepath, &buf);
+               if (rval < 0)
+               {
+                   fprintf(stderr, "Unable to get file status: %s\n",
+                           strerror(rval)); /* warning only */
+               }
             }

             iterator->timestamp = file_timestamp;

In directory 'snort-2.9.7.0-alpha/src/preprocessors',
file 'perf-base.c', I found a call to fseek without a
corresponding check for a return value of < 0, indicating
failure.  The patch file below corrects this issue:

--- perf-base.c.orig    2014-03-11 19:31:04.498623663 -0700
+++ perf-base.c 2014-03-11 19:37:46.668737416 -0700
@@ -1433,7 +1433,11 @@
         WarningMessage("%s: Failed to write stats\n", __FUNCTION__);

         // fseek to adjust offset; ftruncate doesn't do that for us.
-        fseek(fh, start, SEEK_SET);
+        wrote = fseek(fh, start, SEEK_SET);
+       if (wrote == -1)
+       {
+           WarningMessage("%s: Failed to adjust offset\n", __FUNCTION__);
+       }
         ftruncate(fileno(fh), start);
     }

In directory "snort-2.9.7.0.alpha/src/dynamic-preprocessors/appid"
file 'luaDetectorModule.c', I found calls to fseek() and ftell()
without proper checks for a return value of < 0, which would
be an error.  The patch file below adds the necessary checks:

--- luaDetectorModule.c.orig    2014-03-11 19:40:14.065470783 -0700
+++ luaDetectorModule.c 2014-03-11 19:47:25.833346260 -0700
@@ -1037,9 +1037,24 @@
         }

         /*Load lua file as a detector. */
-        fseek(file, 0, SEEK_END);
+        rval = fseek(file, 0, SEEK_END);
+       if (rval == -1)
+       {
+           _dpd.errMsg("Unable to seek lua detector '%s'\n",globs.gl_pathv[n]);
+           continue;
+       }
         validatorBufferLen = ftell(file);
-        fseek(file, 0, SEEK_SET);
+       if (validatorBufferLen == -1)
+       {
+           _dpd.errMsg("Unable to return offset on lua detector '%s'\n",globs.gl_pathv[n]);
+           continue;
+       }
+        rval = fseek(file, 0, SEEK_SET);
+       if (rval == -1)
+       {
+           _dpd.errMsg("Unable to seek lua detector '%s'\n",globs.gl_pathv[n]);
+           continue;
+       }

         if ((validatorBuffer = malloc(validatorBufferLen + 1)) == NULL)
         {

I am attaching the patch file(s) to this email.

A 'make' results in a clean compile of the above patch file(s).

Bill Parker (wp02855 at gmail dot com)

Ho, Ha, Ha, Guard, Turn, Parry, Dodge, Spin, Ha, Thrust! (Robin Hood Daffy)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.snort.org/pipermail/snort-devel/attachments/20140312/0f035bd1/attachment.html>


More information about the Snort-devel mailing list