tools: use util_strtonum for options checking

Function atoi is not safe since miss validation;
Function strtol is better but need to consider empty string and overflows
Function util_strtonum is a safer wrapper of strtoll

Use util_strtonum to check nodeid option and strict checking condition.

Signed-off-by: liangxin1300 <XLiang@suse.com>
Reviewed-by: Jan Friesse <jfriesse@redhat.com>

diff --git a/tools/Makefile.am b/tools/Makefile.am
index 079388474a385b281b51a5d256688a440cb9b3a2..551aa21f30cd4e8e9c5a7fc63f9db4d8c3361e4c 100644 (file)
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -42,12 +42,18 @@ if INSTALL_XMLCONF
 bin_SCRIPTS            += corosync-xmlproc
 endif
 
+noinst_HEADERS          = util.h
+
 AM_CFLAGS              = $(knet_CFLAGS)
 
 EXTRA_DIST             = corosync-xmlproc.sh \
                          corosync-notifyd.sysconfig.example \
                           corosync-blackbox.sh
 
+corosync_cfgtool_SOURCES = corosync-cfgtool.c util.c
+
+corosync_quorumtool_SOURCES = corosync-quorumtool.c util.c
+
 corosync-xmlproc: corosync-xmlproc.sh
        $(SED) -e 's#@''DATADIR@#${datadir}#g' \
               -e 's#@''BASHPATH@#${BASHPATH}#g' \

diff --git a/tools/corosync-cfgtool.c b/tools/corosync-cfgtool.c
index 366cc4ac8cb90328f85e44c30bcfcc287b07f53f..48a35a7032b6e0b3c6f7a5b382e123dfd460c81e 100644 (file)
--- a/tools/corosync-cfgtool.c
+++ b/tools/corosync-cfgtool.c
@@ -46,11 +46,13 @@
 #include <sys/un.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
+#include <limits.h>
 
 #include <corosync/corotypes.h>
 #include <corosync/totem/totem.h>
 #include <corosync/cfg.h>
 #include <corosync/cmap.h>
+#include "util.h"
 
 #define cs_repeat(result, max, code)                           \
        do {                                                    \
@@ -452,6 +454,7 @@ int main (int argc, char *argv[]) {
        int rc = EXIT_SUCCESS;
        enum user_action action = ACTION_NOOP;
        int brief = 0;
+       long long int l;
 
        while ( (opt = getopt(argc, argv, options)) != -1 ) {
                switch (opt) {
@@ -472,14 +475,22 @@ int main (int argc, char *argv[]) {
                        action = ACTION_REOPEN_LOG_FILES;
                        break;
                case 'k':
-                       nodeid = atoi (optarg);
+                       if (util_strtonum(optarg, 1, UINT_MAX, &l) == -1) {
+                               fprintf(stderr, "The nodeid was not valid, try a positive number\n");
+                               exit(EXIT_FAILURE);
+                       }
+                       nodeid = l;
                        action = ACTION_KILL_NODE;
                        break;
                case 'H':
                        action = ACTION_SHUTDOW;
                        break;
                case 'a':
-                       nodeid = atoi (optarg);
+                       if (util_strtonum(optarg, 1, UINT_MAX, &l) == -1) {
+                               fprintf(stderr, "The nodeid was not valid, try a positive number\n");
+                               exit(EXIT_FAILURE);
+                       }
+                       nodeid = l;
                        action = ACTION_SHOWADDR;
                        break;
                case '?':

diff --git a/tools/corosync-quorumtool.c b/tools/corosync-quorumtool.c
index 44bf181cb448241649cab219bbba36afe571015d..1eb968715b35be0f4565f247b654e066b9fa5280 100644 (file)
--- a/tools/corosync-quorumtool.c
+++ b/tools/corosync-quorumtool.c
@@ -48,6 +48,7 @@
 #include <corosync/cmap.h>
 #include <corosync/quorum.h>
 #include <corosync/votequorum.h>
+#include "util.h"
 
 typedef enum {
        NODEID_FORMAT_DECIMAL,
@@ -883,7 +884,6 @@ static void close_all(void) {
 
 int main (int argc, char *argv[]) {
        const char *options = "VHaslpmfe:v:hin:o:";
-       char *endptr;
        int opt;
        int votes = 0;
        int ret = 0;
@@ -894,7 +894,7 @@ int main (int argc, char *argv[]) {
        command_t command_opt = CMD_SHOWSTATUS;
        sorttype_t sort_opt = SORT_ADDR;
        char sortchar;
-       long int l;
+       long long int l;
 
        if (init_all()) {
                close_all();
@@ -934,11 +934,11 @@ int main (int argc, char *argv[]) {
                        break;
                case 'e':
                        if (using_votequorum() > 0) {
-                               votes = strtol(optarg, &endptr, 0);
-                               if ((votes == 0 && endptr == optarg) || votes <= 0) {
+                               if (util_strtonum(optarg, 1, INT_MAX, &l) == -1) {
                                        fprintf(stderr, "New expected votes value was not valid, try a positive number\n");
                                        exit(EXIT_FAILURE);
                                } else {
+                                       votes = l;
                                        command_opt = CMD_SETEXPECTED;
                                }
                        } else {
@@ -947,8 +947,7 @@ int main (int argc, char *argv[]) {
                        }
                        break;
                case 'n':
-                       l = strtol(optarg, &endptr, 0);
-                       if ((l == 0 && endptr == optarg) || l < 0) {
+                       if (util_strtonum(optarg, 1, UINT_MAX, &l) == -1) {
                                fprintf(stderr, "The nodeid was not valid, try a positive number\n");
                                exit(EXIT_FAILURE);
                        }
@@ -957,11 +956,11 @@ int main (int argc, char *argv[]) {
                        break;
                case 'v':
                        if (using_votequorum() > 0) {
-                               votes = strtol(optarg, &endptr, 0);
-                               if ((votes == 0 && endptr == optarg) || votes < 0) {
+                               if (util_strtonum(optarg, 0, INT_MAX, &l) == -1) {
                                        fprintf(stderr, "New votes value was not valid, try a positive number or zero\n");
                                        exit(EXIT_FAILURE);
                                } else {
+                                       votes = l;
                                        command_opt = CMD_SETVOTES;
                                }
                        }

diff --git a/tools/util.c b/tools/util.c
new file mode 100644 (file)
index 0000000..f2ef5fd
--- /dev/null
+++ b/tools/util.c
@@ -0,0 +1,35 @@
+#include <stdlib.h>
+#include <errno.h>
+
+#include "util.h"
+
+/*
+ * Safer wrapper of strtoll. Return 0 on success, otherwise -1.
+ * Idea from corosync-qdevice project
+ */
+int
+util_strtonum(const char *str, long long int min_val, long long int max_val,
+    long long int *res)
+{
+        long long int tmp_ll;
+        char *ep;
+
+        if (min_val > max_val) {
+                return (-1);
+        }
+
+        errno = 0;
+
+        tmp_ll = strtoll(str, &ep, 10);
+        if (ep == str || *ep != '\0' || errno != 0) {
+                return (-1);
+        }
+
+        if (tmp_ll < min_val || tmp_ll > max_val) {
+                return (-1);
+        }
+
+        *res = tmp_ll;
+
+        return (0);
+}

diff --git a/tools/util.h b/tools/util.h
new file mode 100644 (file)
index 0000000..619cf67
--- /dev/null
+++ b/tools/util.h
@@ -0,0 +1,15 @@
+#ifndef COROSYNC_TOOLS_UTIL_H_DEFINED
+#define COROSYNC_TOOLS_UTIL_H_DEFINED
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+extern int              util_strtonum(const char *str, long long int min_val,
+    long long int max_val, long long int *res);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* COROSYNC_TOOLS_UTIL_H_DEFINED */