]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
conf: multiple NULL-pointer dereferences in StreamTcpInitConfig
authorWolfgang Hotwagner <code@feedyourhead.at>
Mon, 11 Dec 2017 20:20:00 +0000 (20:20 +0000)
committerVictor Julien <victor@inliniac.net>
Tue, 30 Jan 2018 09:32:16 +0000 (10:32 +0100)
There are several NULL-pointer derefs in StreamTCPInitConfig. All of them happen because ConfGet returns 1 even if the value is NULL(due to misconfiguration for example).
This commit introduces a new function "ConfGetValue". It adds return values for NULL-pointer to ConfGet and could be used as a replacement for ConfGet.

Note: Simply modify ConfGet might not be a good idea, because there are some places where ConfGet should return 1 even if "value" is NULL. For example if ConfGet should get a Config-Leave in the yaml-hierarchy.

Bug: 2354

src/conf.c
src/conf.h
src/stream-tcp.c

index 2234db72db0657216eeafd7fc15b25fca69df7fd..dfd2410df7703c563e9ba206982e9761385042f3 100644 (file)
@@ -341,6 +341,50 @@ int ConfGet(const char *name, const char **vptr)
     }
 }
 
+/**
+ * \brief Retrieve the value of a configuration node.
+ *
+ * This function will return the value for a configuration node based
+ * on the full name of the node. This function notifies if vptr returns NULL
+ * or if name is set to NULL.
+ *
+ * \param name Name of configuration parameter to get.
+ * \param vptr Pointer that will be set to the configuration value parameter.
+ *   Note that this is just a reference to the actual value, not a copy.
+ *
+ * \retval 0 will be returned if name was not found,
+ *    1 will be returned if the name and it's value was found,
+ *   -1 if the value returns NULL,
+ *   -2 if name is NULL.
+ */
+int ConfGetValue(const char *name, const char **vptr)
+{
+    ConfNode *node;
+
+    if (name == NULL) {
+        SCLogError(SC_ERR_INVALID_ARGUMENT,"parameter 'name' is NULL");
+        return -2;
+    }
+
+    node = ConfGetNode(name);
+
+    if (node == NULL) {
+        SCLogDebug("failed to lookup configuration parameter '%s'", name);
+        return 0;
+    }
+    else {
+
+        if (node->val == NULL) {
+            SCLogDebug("value for configuration parameter '%s' is NULL", name);
+            return -1;
+        }
+
+        *vptr = node->val;
+        return 1;
+    }
+
+}
+
 int ConfGetChildValue(const ConfNode *base, const char *name, const char **vptr)
 {
     ConfNode *node = ConfNodeLookupChild(base, name);
index f831bf8da94253fbfaedc12cdc4b959759dfb6bc..171cc83db5a5aba0c7209e9ee1595df7f61a1834 100644 (file)
@@ -57,6 +57,7 @@ void ConfInit(void);
 void ConfDeInit(void);
 ConfNode *ConfGetRootNode(void);
 int ConfGet(const char *name, const char **vptr);
+int ConfGetValue(const char *name, const char **vptr);
 int ConfGetInt(const char *name, intmax_t *val);
 int ConfGetBool(const char *name, int *val);
 int ConfGetDouble(const char *name, double *val);
index a2af379cad579e00f3a70c30ee6bc76d4bf8a91a..140cbfbab4886a89ef50569cb153b6c9940ad53a 100644 (file)
@@ -363,7 +363,7 @@ void StreamTcpInitConfig(char quiet)
     }
 
     const char *temp_stream_memcap_str;
-    if (ConfGet("stream.memcap", &temp_stream_memcap_str) == 1) {
+    if (ConfGetValue("stream.memcap", &temp_stream_memcap_str) == 1) {
         if (ParseSizeStringU64(temp_stream_memcap_str, &stream_config.memcap) < 0) {
             SCLogError(SC_ERR_SIZE_PARSE, "Error parsing stream.memcap "
                        "from conf file - %s.  Killing engine",
@@ -408,7 +408,7 @@ void StreamTcpInitConfig(char quiet)
     }
 
     const char *temp_stream_inline_str;
-    if (ConfGet("stream.inline", &temp_stream_inline_str) == 1) {
+    if (ConfGetValue("stream.inline", &temp_stream_inline_str) == 1) {
         int inl = 0;
 
         /* checking for "auto" and falling back to boolean to provide
@@ -471,7 +471,7 @@ void StreamTcpInitConfig(char quiet)
     }
 
     const char *temp_stream_reassembly_memcap_str;
-    if (ConfGet("stream.reassembly.memcap", &temp_stream_reassembly_memcap_str) == 1) {
+    if (ConfGetValue("stream.reassembly.memcap", &temp_stream_reassembly_memcap_str) == 1) {
         if (ParseSizeStringU64(temp_stream_reassembly_memcap_str,
                                &stream_config.reassembly_memcap) < 0) {
             SCLogError(SC_ERR_SIZE_PARSE, "Error parsing "
@@ -489,7 +489,7 @@ void StreamTcpInitConfig(char quiet)
     }
 
     const char *temp_stream_reassembly_depth_str;
-    if (ConfGet("stream.reassembly.depth", &temp_stream_reassembly_depth_str) == 1) {
+    if (ConfGetValue("stream.reassembly.depth", &temp_stream_reassembly_depth_str) == 1) {
         if (ParseSizeStringU32(temp_stream_reassembly_depth_str,
                                &stream_config.reassembly_depth) < 0) {
             SCLogError(SC_ERR_SIZE_PARSE, "Error parsing "
@@ -516,7 +516,7 @@ void StreamTcpInitConfig(char quiet)
 
     if (randomize) {
         const char *temp_rdrange;
-        if (ConfGet("stream.reassembly.randomize-chunk-range",
+        if (ConfGetValue("stream.reassembly.randomize-chunk-range",
                     &temp_rdrange) == 1) {
             if (ParseSizeStringU16(temp_rdrange, &rdrange) < 0) {
                 SCLogError(SC_ERR_SIZE_PARSE, "Error parsing "
@@ -534,7 +534,7 @@ void StreamTcpInitConfig(char quiet)
     }
 
     const char *temp_stream_reassembly_toserver_chunk_size_str;
-    if (ConfGet("stream.reassembly.toserver-chunk-size",
+    if (ConfGetValue("stream.reassembly.toserver-chunk-size",
                 &temp_stream_reassembly_toserver_chunk_size_str) == 1) {
         if (ParseSizeStringU16(temp_stream_reassembly_toserver_chunk_size_str,
                                &stream_config.reassembly_toserver_chunk_size) < 0) {
@@ -556,7 +556,7 @@ void StreamTcpInitConfig(char quiet)
                    (r * 1.0 / RAND_MAX - 0.5) * rdrange / 100);
     }
     const char *temp_stream_reassembly_toclient_chunk_size_str;
-    if (ConfGet("stream.reassembly.toclient-chunk-size",
+    if (ConfGetValue("stream.reassembly.toclient-chunk-size",
                 &temp_stream_reassembly_toclient_chunk_size_str) == 1) {
         if (ParseSizeStringU16(temp_stream_reassembly_toclient_chunk_size_str,
                                &stream_config.reassembly_toclient_chunk_size) < 0) {