From: Shivani Bhardwaj Date: Sat, 21 Dec 2019 08:37:58 +0000 (+0530) Subject: src: remove multiple uses of atoi X-Git-Tag: suricata-6.0.0-beta1~327 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=2162c52b17ce09c76cc900ec45b4b05fb87095a7;p=thirdparty%2Fsuricata.git src: remove multiple uses of atoi atoi() and related functions lack a mechanism for reporting errors for invalid values. Replace them with calls to the appropriate ByteExtractString* and StringParse* functions. Partially closes redmine ticket #3053. --- diff --git a/src/util-affinity.c b/src/util-affinity.c index c0216c5c80..0fcafa0d04 100644 --- a/src/util-affinity.c +++ b/src/util-affinity.c @@ -26,6 +26,7 @@ #define _THREAD_AFFINITY #include "util-affinity.h" #include "util-cpu.h" +#include "util-byte.h" #include "conf.h" #include "threads.h" #include "queue.h" @@ -280,7 +281,10 @@ void AffinitySetupLoadFromConfig() node = ConfNodeLookupChild(affinity->head.tqh_first, "threads"); if (node != NULL) { - taf->nb_threads = atoi(node->val); + if (StringParseUint32(&taf->nb_threads, 10, 0, (const char *)node->val) < 0) { + FatalError(SC_ERR_INVALID_ARGUMENT, "invalid value for threads " + "count: '%s'", node->val); + } if (! taf->nb_threads) { SCLogError(SC_ERR_INVALID_ARGUMENT, "bad value for threads count"); exit(EXIT_FAILURE); diff --git a/src/util-classification-config.c b/src/util-classification-config.c index c71ede6cc3..6006973734 100644 --- a/src/util-classification-config.c +++ b/src/util-classification-config.c @@ -34,6 +34,7 @@ #include "util-error.h" #include "util-debug.h" #include "util-fmemopen.h" +#include "util-byte.h" /* Regex to parse the classtype argument from a Signature. The first substring * holds the classtype name, the second substring holds the classtype the @@ -244,7 +245,7 @@ int SCClassConfAddClasstype(DetectEngineCtx *de_ctx, char *rawstr, uint16_t inde char ct_name[CLASSTYPE_NAME_MAX_LEN]; char ct_desc[CLASSTYPE_DESC_MAX_LEN]; char ct_priority_str[16]; - int ct_priority = 0; + uint32_t ct_priority = 0; uint16_t ct_id = index; SCClassConfClasstype *ct_new = NULL; @@ -281,12 +282,10 @@ int SCClassConfAddClasstype(DetectEngineCtx *de_ctx, char *rawstr, uint16_t inde SCLogInfo("pcre_copy_substring() failed"); goto error; } - if (strlen(ct_priority_str) == 0) { + if (StringParseUint32(&ct_priority, 10, 0, (const char *)ct_priority_str) < 0) { goto error; } - ct_priority = atoi(ct_priority_str); - /* Create a new instance of the parsed Classtype string */ ct_new = SCClassConfAllocClasstype(ct_id, ct_name, ct_desc, ct_priority); if (ct_new == NULL) diff --git a/src/util-cpu.c b/src/util-cpu.c index f21a338879..068e8dbe3b 100644 --- a/src/util-cpu.c +++ b/src/util-cpu.c @@ -27,6 +27,7 @@ #include "util-error.h" #include "util-debug.h" #include "util-cpu.h" +#include "util-byte.h" /** * Ok, if they should use sysconf, check that they have the macro's @@ -75,9 +76,15 @@ uint16_t UtilCpuGetNumProcessorsConfigured(void) return (uint16_t)nprocs; #elif OS_WIN32 - long nprocs = -1; - const char* envvar = getenv("NUMBER_OF_PROCESSORS"); - nprocs = (NULL != envvar) ? atoi(envvar) : 0; + int64_t nprocs = 0; + const char* envvar = getenv("NUMBER_OF_PROCESSORS"); + if (envvar != NULL) { + if (StringParseInt64(&nprocs, 10, 0, envvar) < 0) { + SCLogWarning(SC_ERR_INVALID_VALUE, "Invalid value for number of " + "processors: %s", envvar); + return 0; + } + } if (nprocs < 1) { SCLogError(SC_ERR_SYSCALL, "Couldn't retrieve the number of cpus " "configured from the NUMBER_OF_PROCESSORS environment variable"); diff --git a/src/util-host-info.c b/src/util-host-info.c index dba9fad1c0..f481c41a6a 100644 --- a/src/util-host-info.c +++ b/src/util-host-info.c @@ -27,6 +27,7 @@ #include "suricata-common.h" #include "config.h" #include "util-host-info.h" +#include "util-byte.h" #ifndef OS_WIN32 #include @@ -83,13 +84,23 @@ int SCKernelVersionIsAtLeast(int major, int minor) pcre_get_substring_list(kuname.release, ov, ret, &list); - kmajor = atoi(list[1]); - kminor = atoi(list[2]); + bool err = false; + if (StringParseInt32(&kmajor, 10, 0, (const char *)list[1]) < 0) { + SCLogError(SC_ERR_INVALID_VALUE, "Invalid value for kmajor: '%s'", list[1]); + err = true; + } + if (StringParseInt32(&kminor, 10, 0, (const char *)list[2]) < 0) { + SCLogError(SC_ERR_INVALID_VALUE, "Invalid value for kminor: '%s'", list[2]); + err = true; + } pcre_free_substring_list(list); pcre_free_study(version_regex_study); pcre_free(version_regex); + if (err) + goto error; + if (kmajor > major) return 1; if (kmajor == major && kminor >= minor) diff --git a/src/util-host-os-info.c b/src/util-host-os-info.c index 4ed06b7d93..55b06ea70c 100644 --- a/src/util-host-os-info.c +++ b/src/util-host-os-info.c @@ -29,6 +29,7 @@ #include "util-debug.h" #include "util-ip.h" #include "util-radix-tree.h" +#include "util-byte.h" #include "stream-tcp-private.h" #include "stream-tcp-reassemble.h" @@ -186,8 +187,7 @@ int SCHInfoAddHostOSInfo(const char *host_os, const char *host_os_ip_range, int SCRadixAddKeyIPV4((uint8_t *)ipv4_addr, sc_hinfo_tree, (void *)user_data); } else { - netmask_value = atoi(netmask_str); - if (netmask_value < 0 || netmask_value > 32) { + if (StringParseI32RangeCheck(&netmask_value, 10, 0, (const char *)netmask_str, 0, 32) < 0) { SCLogError(SC_ERR_INVALID_IP_NETBLOCK, "Invalid IPV4 Netblock"); SCHInfoFreeUserDataOSPolicy(user_data); SCFree(ipv4_addr); @@ -212,8 +212,7 @@ int SCHInfoAddHostOSInfo(const char *host_os, const char *host_os_ip_range, int SCRadixAddKeyIPV6((uint8_t *)ipv6_addr, sc_hinfo_tree, (void *)user_data); } else { - netmask_value = atoi(netmask_str); - if (netmask_value < 0 || netmask_value > 128) { + if (StringParseI32RangeCheck(&netmask_value, 10, 0, (const char *)netmask_str, 0, 128) < 0) { SCLogError(SC_ERR_INVALID_IP_NETBLOCK, "Invalid IPV6 Netblock"); SCHInfoFreeUserDataOSPolicy(user_data); SCFree(ipv6_addr); diff --git a/src/util-ip.c b/src/util-ip.c index be127327ed..293e55527b 100644 --- a/src/util-ip.c +++ b/src/util-ip.c @@ -26,6 +26,7 @@ #include "suricata-common.h" #include "util-ip.h" +#include "util-byte.h" /** \brief determine if a string is a valid ipv4 address * \retval bool is addr valid? @@ -65,9 +66,9 @@ bool IPv4AddressStringIsValid(const char *str) addr[dots][alen] = '\0'; for (int x = 0; x < 4; x++) { - int a = atoi(addr[x]); - if (a < 0 || a >= 256) { - SCLogDebug("out of range"); + uint8_t a; + if (StringParseUint8(&a, 10, 0, (const char *)addr[x]) < 0) { + SCLogDebug("invalid value for address byte: %s", addr[x]); return false; } } diff --git a/src/util-log-redis.c b/src/util-log-redis.c index a145e4f564..2dc1d86943 100644 --- a/src/util-log-redis.c +++ b/src/util-log-redis.c @@ -26,6 +26,7 @@ #include "suricata-common.h" /* errno.h, string.h, etc. */ #include "util-log-redis.h" #include "util-logopenfile.h" +#include "util-byte.h" #ifdef HAVE_LIBHIREDIS @@ -534,7 +535,9 @@ int SCConfLogOpenRedis(ConfNode *redis_node, void *lf_ctx) SCLogError(SC_ERR_MEM_ALLOC, "Error allocating redis server string"); exit(EXIT_FAILURE); } - log_ctx->redis_setup.port = atoi(redis_port); + if (StringParseUint16(&log_ctx->redis_setup.port, 10, 0, (const char *)redis_port) < 0) { + FatalError(SC_ERR_INVALID_VALUE, "Invalid value for redis port: %s", redis_port); + } log_ctx->Close = SCLogFileCloseRedis; #ifdef HAVE_LIBEVENT diff --git a/src/util-napatech.c b/src/util-napatech.c index 710131b14c..9e97981f7e 100644 --- a/src/util-napatech.c +++ b/src/util-napatech.c @@ -768,10 +768,20 @@ static uint32_t CountWorkerThreads(void) char copystr[16]; strlcpy(copystr, lnode->val, 16); - - ByteExtractStringUint8(&start, 10, 0, copystr); - ByteExtractStringUint8(&end, 10, 0, strchr(copystr, '-') + 1); - + if (StringParseUint8(&start, 10, 0, (const char *)copystr) < 0) { + FatalError(SC_ERR_INVALID_VALUE, "Napatech invalid" + " worker range start: '%s'", copystr); + } + char *end_str = strchr(copystr, '-'); + if ((end_str == NULL) || (end_str != NULL && StringParseUint8(&end, + 10, 0, (const char *) (end_str + 1)) < 0)) { + FatalError(SC_ERR_INVALID_VALUE, "Napatech invalid" + " worker range end: '%s'", (end_str != NULL) ? (const char *)(end_str + 1) : "Null"); + } + if (end < start) { + FatalError(SC_ERR_INVALID_VALUE, "Napatech invalid" + " worker range start: '%s' is greater than end: '%s'", start, end); + } worker_count = end - start + 1; } else { @@ -813,8 +823,8 @@ int NapatechGetStreamConfig(NapatechStreamConfig stream_config[]) int set_cpu_affinity = 0; ConfNode *ntstreams; uint16_t stream_id = 0; - uint16_t start = 0; - uint16_t end = 0; + uint8_t start = 0; + uint8_t end = 0; for (uint16_t i = 0; i < MAX_STREAMS; ++i) { stream_config[i].stream_id = 0; @@ -916,8 +926,16 @@ int NapatechGetStreamConfig(NapatechStreamConfig stream_config[]) char copystr[16]; strlcpy(copystr, stream->val, 16); - ByteExtractStringUint16(&start, 10, 0, copystr); - ByteExtractStringUint16(&end, 10, 0, strchr(copystr, '-') + 1); + if (StringParseUint8(&start, 10, 0, (const char *)copystr) < 0) { + FatalError(SC_ERR_INVALID_VALUE, "Napatech invalid " + "stream id start: '%s'", copystr); + } + char *end_str = strchr(copystr, '-'); + if ((end_str == NULL) || (end_str != NULL && StringParseUint8(&end, + 10, 0, (const char *) (end_str + 1)) < 0)) { + FatalError(SC_ERR_INVALID_VALUE, "Napatech invalid " + "stream id end: '%s'", (end_str != NULL) ? (const char *)(end_str + 1) : "Null"); + } } else { if (stream_spec == CONFIG_SPECIFIER_RANGE) { SCLogError(SC_ERR_NAPATECH_PARSE_CONFIG, @@ -925,7 +943,11 @@ int NapatechGetStreamConfig(NapatechStreamConfig stream_config[]) exit(EXIT_FAILURE); } stream_spec = CONFIG_SPECIFIER_INDIVIDUAL; - ByteExtractStringUint16(&stream_config[instance_cnt].stream_id, 10, 0, stream->val); + if (StringParseUint16(&stream_config[instance_cnt].stream_id, + 10, 0, (const char *)stream->val) < 0) { + FatalError(SC_ERR_INVALID_VALUE, "Napatech invalid " + "stream id: '%s'", stream->val); + } start = stream_config[instance_cnt].stream_id; end = stream_config[instance_cnt].stream_id; } diff --git a/src/util-privs.c b/src/util-privs.c index 8c2c4ce89b..ff3238521d 100644 --- a/src/util-privs.c +++ b/src/util-privs.c @@ -31,6 +31,7 @@ #include "suricata.h" #include "util-privs.h" +#include "util-byte.h" #ifdef HAVE_LIBCAP_NG @@ -154,7 +155,9 @@ int SCGetUserID(const char *user_name, const char *group_name, uint32_t *uid, ui /* Get the user ID */ if (isdigit((unsigned char)user_name[0]) != 0) { - userid = atoi(user_name); + if (ByteExtractStringUint32(&userid, 10, 0, (const char *)user_name) < 0) { + FatalError(SC_ERR_UID_FAILED, "invalid user id value: '%s'", user_name); + } pw = getpwuid(userid); if (pw == NULL) { SCLogError(SC_ERR_UID_FAILED, "unable to get the user ID, " @@ -176,7 +179,9 @@ int SCGetUserID(const char *user_name, const char *group_name, uint32_t *uid, ui struct group *gp; if (isdigit((unsigned char)group_name[0]) != 0) { - groupid = atoi(group_name); + if (ByteExtractStringUint32(&groupid, 10, 0, (const char *)group_name) < 0) { + FatalError(SC_ERR_GID_FAILED, "invalid group id: '%s'", group_name); + } } else { gp = getgrnam(group_name); if (gp == NULL) { @@ -216,7 +221,9 @@ int SCGetGroupID(const char *group_name, uint32_t *gid) /* Get the group ID */ if (isdigit((unsigned char)group_name[0]) != 0) { - grpid = atoi(group_name); + if (ByteExtractStringUint32(&grpid, 10, 0, (const char *)group_name) < 0) { + FatalError(SC_ERR_GID_FAILED, "invalid group id: '%s'", group_name); + } } else { gp = getgrnam(group_name); if (gp == NULL) { diff --git a/src/util-proto-name.c b/src/util-proto-name.c index 2392682d7b..f9b00f053c 100644 --- a/src/util-proto-name.c +++ b/src/util-proto-name.c @@ -26,6 +26,7 @@ #include "suricata-common.h" #include "util-proto-name.h" +#include "util-byte.h" /** Lookup array to hold the information related to known protocol * in /etc/protocols */ @@ -60,8 +61,8 @@ void SCProtoNameInit() if (proto_ch == NULL) continue; - int proto = atoi(proto_ch); - if (proto >= 255) + uint8_t proto; + if (StringParseUint8(&proto, 10, 0, (const char *)proto_ch) < 0) continue; char *cname = strtok_r(NULL, " \t", &ptr); diff --git a/src/util-radix-tree.c b/src/util-radix-tree.c index 170d9ee9d8..583c1003b4 100644 --- a/src/util-radix-tree.c +++ b/src/util-radix-tree.c @@ -30,6 +30,7 @@ #include "util-ip.h" #include "util-unittest.h" #include "util-memcmp.h" +#include "util-byte.h" /** * \brief Allocates and returns a new instance of SCRadixUserData. @@ -947,7 +948,7 @@ SCRadixNode *SCRadixAddKeyIPV4String(const char *str, SCRadixTree *tree, void *u /* Does it have a mask? */ if (NULL != (mask_str = strchr(ip_str, '/'))) { - int cidr; + uint8_t cidr; *(mask_str++) = '\0'; /* Dotted type netmask not supported (yet) */ @@ -956,10 +957,10 @@ SCRadixNode *SCRadixAddKeyIPV4String(const char *str, SCRadixTree *tree, void *u } /* Get binary values for cidr mask */ - cidr = atoi(mask_str); - if ((cidr < 0) || (cidr > 32)) { + if (StringParseU8RangeCheck(&cidr, 10, 0, (const char *)mask_str, 0, 32) < 0) { return NULL; } + netmask = (uint8_t)cidr; } @@ -995,7 +996,7 @@ SCRadixNode *SCRadixAddKeyIPV6String(const char *str, SCRadixTree *tree, void *u /* Does it have a mask? */ if (NULL != (mask_str = strchr(ip_str, '/'))) { - int cidr; + uint8_t cidr; *(mask_str++) = '\0'; /* Dotted type netmask not supported (yet) */ @@ -1004,10 +1005,10 @@ SCRadixNode *SCRadixAddKeyIPV6String(const char *str, SCRadixTree *tree, void *u } /* Get binary values for cidr mask */ - cidr = atoi(mask_str); - if ((cidr < 0) || (cidr > 128)) { + if (StringParseU8RangeCheck(&cidr, 10, 0, (const char *)mask_str, 0, 128) < 0) { return NULL; } + netmask = (uint8_t)cidr; }