usage: be careful when parsing numeric user input

Functions atoi(3) and atof(3) do not perform any error checking.  Use
strtol(3) function family, with alterations to fail if input is not
expected.

diff --git a/curses.c b/curses.c
index 4caa8fcdca8b998edbac189a6a5a0995de54364c..cac0aef3ebf77d3e957dfab949da18c6ee3c90ba 100644 (file)
--- a/curses.c
+++ b/curses.c
@@ -566,7 +566,7 @@ void mtr_fill_graph(int at, int cols)
                        printw("%c", '?');
                        attrset(A_NORMAL);
                } else {
-                       if (display_mode == 1) {
+                       if (display_mode == DisplayModeBlockmap) {
                                if (saved[i] > scale[6]) {
                                        printw("%c", block_map[NUM_FACTORS-1]);
                                } else {
@@ -669,7 +669,7 @@ void mtr_curses_redraw(void)
   attron(A_BOLD); printw("O"); attroff(A_BOLD); printw("rder of fields   ");
   attron(A_BOLD); printw("q"); attroff(A_BOLD); printw("uit\n");
   
-  if (display_mode == 0) {
+  if (display_mode == DisplayModeDefalt) {
     for (i=0; i < MAXFLD; i++ ) {
        j = fld_index[fld_active[i]];
        if (j < 0) continue;

diff --git a/display.h b/display.h
index edd2a9938d78d927fa466bd3c6fd4082830c7b94..267eca9f17cbd24764d0334c68e0d1ba69276b8a 100644 (file)
--- a/display.h
+++ b/display.h
@@ -43,6 +43,12 @@ enum {
   DisplayJSON
 };
 
+enum {
+  DisplayModeDefalt,
+  DisplayModeBlockmap,
+  DisplayModeMAX       /* this must be the last DisplayMode entry */
+};
+
 /*  Prototypes for display.c  */
 void display_detect(int *argc, char ***argv);
 void display_open(void);

diff --git a/mtr.c b/mtr.c
index bce7822d51347561590d0e5f6930741f9f8ba8d9..f600682f6a28019eccab6c2b4c56f5b8e5eacc7e 100644 (file)
--- a/mtr.c
+++ b/mtr.c
@@ -26,6 +26,8 @@
 #include <errno.h>
 #include <string.h>
 #include <strings.h>
+#include <error.h>
+#include <values.h>
 
 #include <netdb.h>
 #include <netinet/in.h>
@@ -176,6 +178,42 @@ static void __attribute__((__noreturn__)) usage(FILE *out)
   exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS);
 }
 
+/* Parse string, and return positive signed int. */
+static int
+strtoint_or_err (const char *str, const char *errmesg)
+{
+  unsigned long int num;
+  char *end = NULL;
+
+  if (str != NULL && *str != '\0') {
+      errno = 0;
+      num = strtoul (str, &end, 10);
+      if (errno == 0 && str != end && end != NULL && *end == '\0' &&
+          num < INT_MAX)
+       return num;
+    }
+  error (EXIT_FAILURE, errno, "%s: '%s'", errmesg, str);
+  return 0;
+}
+
+static float
+strtofloat_or_err (const char *str, const char *errmesg)
+{
+  double num;
+  char *end = NULL;
+
+  if (str != NULL && *str != '\0') {
+      errno = 0;
+      num = strtod (str, &end);
+      if (errno == 0 && str != end && end != NULL && *end == '\0' &&
+          num < FLT_MAX)
+       return num;
+    }
+  error (EXIT_FAILURE, errno, "%s: '%s'", errmesg, str);
+  return 0;
+}
+
+
 char *
 trim(char * s) {
 
@@ -440,14 +478,17 @@ void parse_arg (int argc, char **argv)
       break;
 
     case OPT_DISPLAYMODE:
-      display_mode = (atoi (optarg)) % 3;
+      display_mode = strtoint_or_err(optarg, "invalid argument");
+      if ((DisplayModeMAX - 1) < display_mode)
+        error(EXIT_FAILURE, 0, "value out of range (%d - %d): %s",
+              DisplayModeDefalt, (DisplayModeMAX - 1), optarg);
       break;
     case 'c':
-      MaxPing = atoi (optarg);
+      MaxPing = strtoint_or_err(optarg, "invalid argument");
       ForceMaxPing = 1;
       break;
     case 's':
-      cpacketsize = atoi (optarg);
+      cpacketsize = strtoint_or_err(optarg, "invalid argument");
       break;
     case 'a':
       InterfaceAddress = optarg;
@@ -459,7 +500,7 @@ void parse_arg (int argc, char **argv)
       dns = 0;
       break;
     case 'i':
-      WaitTime = atof (optarg);
+      WaitTime = strtofloat_or_err(optarg, "invalid argument");
       if (WaitTime <= 0.0) {
        fprintf (stderr, "mtr: wait time must be positive\n");
        exit(EXIT_FAILURE);
@@ -470,7 +511,7 @@ void parse_arg (int argc, char **argv)
       }
       break;
     case 'f':
-      fstTTL = atoi (optarg);
+      fstTTL = strtoint_or_err(optarg, "invalid argument");
       if (fstTTL > maxTTL) {
        fstTTL = maxTTL;
       }
@@ -482,7 +523,7 @@ void parse_arg (int argc, char **argv)
       read_from_file(argv[0], optarg);
       break;
     case 'm':
-      maxTTL = atoi (optarg);
+      maxTTL = strtoint_or_err(optarg, "invalid argument");
       if (maxTTL > (MaxHost - 1)) {
        maxTTL = MaxHost-1;
       }
@@ -494,7 +535,7 @@ void parse_arg (int argc, char **argv)
       }
       break;
        case 'U':
-               maxUnknown = atoi(optarg);
+                maxUnknown = strtoint_or_err(optarg, "invalid argument");
                if (maxUnknown < 1) {
                        maxUnknown = 1;
                }
@@ -514,19 +555,19 @@ void parse_arg (int argc, char **argv)
       strcpy ((char*)fld_active, optarg);
       break;
     case 'B':
-      bitpattern = atoi (optarg);
+      bitpattern = strtoint_or_err(optarg, "invalid argument");
       if (bitpattern > 255)
        bitpattern = -1;
       break;
     case 'G':
-      GraceTime = atof (optarg);
+      GraceTime = strtofloat_or_err(optarg, "invalid argument");
       if (GraceTime <= 0.0) {
         fprintf (stderr, "mtr: wait time must be positive\n");
         exit(EXIT_FAILURE);
       }
       break;
     case 'Q':
-      tos = atoi (optarg);
+      tos = strtoint_or_err(optarg, "invalid argument");
       if (tos > 255 || tos < 0) {
        /* error message, should do more checking for valid values,
         * details in rfc2474 */
@@ -569,21 +610,21 @@ void parse_arg (int argc, char **argv)
       show_ips = 1;
       break;
     case 'P':
-      remoteport = atoi(optarg);
+      remoteport = strtoint_or_err(optarg, "invalid argument");
       if (remoteport > 65535 || remoteport < 1) {
         fprintf(stderr, "Illegal port number.\n");
         exit(EXIT_FAILURE);
       }
       break;
     case 'L':
-      localport = atoi(optarg);
+      localport = strtoint_or_err(optarg, "invalid argument");
       if (localport > 65535 || localport < MinPort) {
         fprintf(stderr, "Illegal local port number.\n");
         exit(EXIT_FAILURE);
       }
       break;
     case 'Z':
-      tcp_timeout = atoi(optarg);
+      tcp_timeout = strtoint_or_err(optarg, "invalid argument");
       tcp_timeout *= 1000000;
       break;
     case '4':
@@ -599,7 +640,7 @@ void parse_arg (int argc, char **argv)
 #endif
 #ifdef HAVE_IPINFO
     case 'y':
-      ipinfo_no = atoi (optarg);
+      ipinfo_no = strtoint_or_err(optarg, "invalid argument");
       if (ipinfo_no < 0)
         ipinfo_no = 0;
       break;
@@ -614,11 +655,7 @@ void parse_arg (int argc, char **argv)
 #endif
 #ifdef SO_MARK
     case 'M':
-      mark = atoi (optarg);
-      if (mark < 0) {
-        fprintf( stderr, "SO_MARK must be positive.\n" );
-        exit(EXIT_FAILURE);
-      }
+      mark = strtoint_or_err(optarg, "invalid argument");
       break;
 #else
     case 'M':
@@ -701,7 +738,7 @@ int main(int argc, char **argv)
   srand (getpid());
 
   display_detect(&argc, &argv);
-  display_mode = 0;
+  display_mode = DisplayModeDefalt;
 
   /* The field options are now in a static array all together,
      but that requires a run-time initialization. */

diff --git a/select.c b/select.c
index 54b7a399756bf17c88efd87c8743ac0ae340de77..64aa8734ae1d6cfcb3cc08171d775b67f3b7b600 100644 (file)
--- a/select.c
+++ b/select.c
@@ -216,7 +216,7 @@ void select_loop(void) {
        net_reset();
        break;
       case ActionDisplay:
-        display_mode = (display_mode+1) % 3;
+        display_mode = (display_mode + 1) % DisplayModeMAX;
        break;
       case ActionClear:
        display_clear();