From: Sami Kerola Date: Sun, 21 Aug 2016 10:49:49 +0000 (+0100) Subject: usage: be careful when parsing numeric user input X-Git-Tag: v0.88~29^2~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2e4369a61c5bf5a6c221a49350e71f8dfebba539;p=thirdparty%2Fmtr.git 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 4caa8fc..cac0aef 100644 --- 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 edd2a99..267eca9 100644 --- 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 bce7822..f600682 100644 --- a/mtr.c +++ b/mtr.c @@ -26,6 +26,8 @@ #include #include #include +#include +#include #include #include @@ -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 54b7a39..64aa873 100644 --- 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();