From: Wolfgang Hotwagner Date: Mon, 11 Dec 2017 20:20:00 +0000 (+0000) Subject: conf: multiple NULL-pointer dereferences in StreamTcpInitConfig X-Git-Tag: suricata-4.0.4~55 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9540efb9d69f6718c221f3ed0b7f22032e768d7c;p=thirdparty%2Fsuricata.git conf: multiple NULL-pointer dereferences in StreamTcpInitConfig 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 --- diff --git a/src/conf.c b/src/conf.c index 2234db72db..dfd2410df7 100644 --- a/src/conf.c +++ b/src/conf.c @@ -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); diff --git a/src/conf.h b/src/conf.h index f831bf8da9..171cc83db5 100644 --- a/src/conf.h +++ b/src/conf.h @@ -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); diff --git a/src/stream-tcp.c b/src/stream-tcp.c index a2af379cad..140cbfbab4 100644 --- a/src/stream-tcp.c +++ b/src/stream-tcp.c @@ -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) {