Missing Sanity Check for sflist_new() in port_table.cc (Snort-3.0.0-a2 (build 172)

Hello All,

In reviewing source code in Snort-3.0.0 alpha 2 (Build 172), in
sub-directory 'src/ports', file 'port_table.cc', in function
'PortTableCompileMergePortObjects()', at line 633, there is a call
to sflist_new() like this:

plx_list = sflist_new();
sflist_init(plx_list);
p->pt_plx_list = plx_list;

without any check for a return value of NULL from sflist_new(), which
could cause other issues, if plx_iist is NULL.

Should the ErrorMessage in the patch be replaced by FatalError?

The patch file below should address/correct this issue:

--- port_table.cc.orig  2015-10-18 14:37:21.264314596 -0700
+++ port_table.cc       2015-10-18 14:42:20.700371379 -0700
@@ -631,6 +631,12 @@
         "***\n*** PortList-Merging, Large Rule groups must have %d
rules\n",p->pt_lrc);

     plx_list = sflist_new();
+    if (plx_list == NULL)
+    {
+       ErrorMessage("Could not allocate new list for port objects\n");
+       return -1;  /*  is plx_list released anywhere in this function? */
+    }
+
     sflist_init(plx_list);

     p->pt_plx_list = plx_list;


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

In sub-directory 'src/test', file 'catch.hpp', there is a call to
malloc() which is NOT checked for a return value of NULL, indicating
failure.  However, according to the comments at the top of this file:

/*
 *  Catch v1.2.1
 *  Generated: 2015-06-30 18:23:27.961086
 *  ----------------------------------------------------------
 *  This file has been merged from multiple headers. Please don't edit it
directly
 *  Copyright (c) 2012 Two Blue Cubes Ltd. All rights reserved.
 *
 *  Distributed under the Boost Software License, Version 1.0. (See
accompanying
 *  file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
 */

This occurs at line 2659, the source code is below:

    inline size_t registerTestMethods() {
        size_t noTestMethods = 0;
        int noClasses = objc_getClassList( NULL, 0 );

        Class* classes = (CATCH_UNSAFE_UNRETAINED Class *)malloc(
sizeof(Class) * noClasses);
        objc_getClassList( classes, noClasses );   <---- this could go
KABOOM, could it not?

        for( int c = 0; c < noClasses; c++ ) {
=======================================================================

I am attaching the patch file 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!

--- port_table.cc.orig	2015-10-18 14:37:21.264314596 -0700
+++ port_table.cc	2015-10-18 14:42:20.700371379 -0700
@@ -631,6 +631,12 @@
         "***\n*** PortList-Merging, Large Rule groups must have %d rules\n",p->pt_lrc);
 
     plx_list = sflist_new();
+    if (plx_list == NULL)
+    {
+	ErrorMessage("Could not allocate new list for port objects\n");
+	return -1;  /*	is plx_list released anywhere in this function? */
+    }
+
     sflist_init(plx_list);
 
     p->pt_plx_list = plx_list;