Replace sscanf with strtoul/strtod for stricter option parsing

diff --git a/ci/test-08-options-n-q.pl b/ci/test-08-options-n-q.pl
index 6fb95356f7adb0b4c77b4253fc8eda2a52b1e2ff..dffa9f222018b857d582b6fe1895b176bf60e24e 100755 (executable)
--- a/ci/test-08-options-n-q.pl
+++ b/ci/test-08-options-n-q.pl
@@ -1,6 +1,6 @@
 #!/usr/bin/perl -w
 
-use Test::Command tests => 48;
+use Test::Command tests => 51;
 use Test::More;
 
 #  -n         show targets by name (-d is equivalent)
@@ -205,3 +205,10 @@ $cmd->stderr_like(qr{\[\d+:\d+:\d+\]
 }
 
 
+# fping -O (hex)
+{
+my $cmd = Test::Command->new(cmd => "fping -O 0x20 --print-tos 127.0.0.1");
+$cmd->exit_is_num(0);
+$cmd->stdout_like(qr{127\.0\.0\.1 is alive \(TOS 32\)}); 
+$cmd->stderr_is_eq("");
+}
\ No newline at end of file

diff --git a/ci/test-12-option-type.pl b/ci/test-12-option-type.pl
index 5b6dac41d56bf10c36487d4f0e215cd45ddc7ba1..51680b5ad21af6a2a619358a708112b937b2ac63 100755 (executable)
--- a/ci/test-12-option-type.pl
+++ b/ci/test-12-option-type.pl
@@ -1,6 +1,6 @@
 #!/usr/bin/perl -w
 
-use Test::Command tests => 90;
+use Test::Command tests => 177;
 use Test::More;
 
 # some options require a numeric argument
@@ -13,6 +13,36 @@ for my $arg (qw(b B c C H i O p Q r t x X -seqmap-timeout)) {
     }
 }
 
+# options that use strtoul and must be strict (no trailing garbage, no zero)
+for my $arg (qw(c C H x X)) {
+    for my $test_input (qw(10abc 0)) {
+        my $cmd = Test::Command->new(cmd => "fping -$arg $test_input");
+        $cmd->exit_is_num(1);
+        $cmd->stdout_is_eq("");
+        $cmd->stderr_like(qr{Usage:});
+    }
+}
+
+# options that use strtoul and must be strict (no trailing garbage)
+for my $arg (qw(r b)) {
+    for my $test_input (qw(10abc)) {
+        my $cmd = Test::Command->new(cmd => "fping -$arg $test_input");
+        $cmd->exit_is_num(1);
+        $cmd->stdout_is_eq("");
+        $cmd->stderr_like(qr{Usage:});
+    }
+}
+
+# options that use strtod and must be strict (no trailing garbage)
+for my $arg (qw(B i p t Q -seqmap-timeout)) {
+    for my $test_input (qw(1.5abc 10abc)) {
+        my $cmd = Test::Command->new(cmd => "fping -$arg $test_input");
+        $cmd->exit_is_num(1);
+        $cmd->stdout_is_eq("");
+        $cmd->stderr_like(qr{Usage:});
+    }
+}
+
 # fping -k, only supported on Linux, requires a number
 SKIP: {
     if($^O ne 'linux') {
@@ -25,3 +55,26 @@ SKIP: {
         $cmd->stderr_like(qr{Usage:});
     }
 }
+
+# fping -k strictness
+SKIP: {
+    if($^O ne 'linux') {
+        skip '-k option is only supported on Linux', 6;
+    }
+    my $arg = 'k';
+    for my $test_input (qw(10abc 0)) {
+        my $cmd = Test::Command->new(cmd => "fping -$arg $test_input 127.0.0.1");
+        $cmd->exit_is_num(1);
+        $cmd->stdout_is_eq("");
+        $cmd->stderr_like(qr{Usage:});
+    }
+}
+
+# fping -g strict mask
+
+for my $target (qw(127.0.0.1/24abc 127.0.0.1/ 127.0.0.1/abc)) {
+    my $cmd = Test::Command->new(cmd => "fping -g $target");
+    $cmd->exit_is_num(1);
+    $cmd->stdout_is_eq("");
+    $cmd->stderr_like(qr{Usage:});
+}

diff --git a/src/fping.c b/src/fping.c
index 8a062f275d399e07dcccad3e2f1e3244a321fb8d..6c7d503ffe18c3a6816f31f44b7142ba02d017d9 100644 (file)
--- a/src/fping.c
+++ b/src/fping.c
@@ -652,7 +652,7 @@ int main(int argc, char **argv)
         { 0, 0, 0 }
     };
 
-    float opt_value_float;
+    double opt_val_double;
     while ((c = optparse_long(&optparse_state, longopts, NULL)) != EOF) {
         switch (c) {
         case '0':
@@ -707,11 +707,13 @@ int main(int argc, char **argv)
                 }
 #endif
             } else if (strstr(optparse_state.optlongname, "seqmap-timeout") != NULL) {
-                if (sscanf(optparse_state.optarg, "%f", &opt_value_float) != 1)
+                errno = 0;
+                opt_val_double = strtod(optparse_state.optarg, &endptr);
+                if (errno != 0 || optparse_state.optarg == endptr || *endptr != '\0')
                     usage(1);
-                if (opt_value_float < 0)
+                if (opt_val_double < 0)
                     usage(1);
-                seqmap_timeout = opt_value_float * 1000000;
+                seqmap_timeout = opt_val_double * 1000000;
             } else {
                 usage(1);
             }
@@ -760,36 +762,44 @@ int main(int argc, char **argv)
             break;
 
         case 't':
-            if (sscanf(optparse_state.optarg, "%f", &opt_value_float) != 1)
+            errno = 0;
+            opt_val_double = strtod(optparse_state.optarg, &endptr);
+            if (errno != 0 || optparse_state.optarg == endptr || *endptr != '\0')
                 usage(1);
-            if (opt_value_float < 0) {
+            if (opt_val_double < 0) {
                 usage(1);
             }
-            timeout = opt_value_float * 1000000;
+            timeout = opt_val_double * 1000000;
             timeout_flag = 1;
             break;
 
         case 'r':
-            if (sscanf(optparse_state.optarg, "%u", &retry) != 1)
+            errno = 0;
+            retry = (unsigned int)strtoul(optparse_state.optarg, &endptr, 10);
+            if (errno != 0 || optparse_state.optarg == endptr || *endptr != '\0')
                 usage(1);
             break;
 
         case 'i':
-            if (sscanf(optparse_state.optarg, "%f", &opt_value_float) != 1)
+            errno = 0;
+            opt_val_double = strtod(optparse_state.optarg, &endptr);
+            if (errno != 0 || optparse_state.optarg == endptr || *endptr != '\0')
                 usage(1);
-            if (opt_value_float < 0) {
+            if (opt_val_double < 0) {
                 usage(1);
             }
-            interval = opt_value_float * 1000000;
+            interval = opt_val_double * 1000000;
             break;
 
         case 'p':
-            if (sscanf(optparse_state.optarg, "%f", &opt_value_float) != 1)
+            errno = 0;
+            opt_val_double = strtod(optparse_state.optarg, &endptr);
+            if (errno != 0 || optparse_state.optarg == endptr || *endptr != '\0')
                 usage(1);
-            if (opt_value_float < 0) {
+            if (opt_val_double < 0) {
                 usage(1);
             }
-            perhost_interval = opt_value_float * 1000000;
+            perhost_interval = opt_val_double * 1000000;
 
             break;
 
@@ -813,7 +823,9 @@ int main(int argc, char **argv)
             break;
 
         case 'b':
-            if (sscanf(optparse_state.optarg, "%u", &ping_data_size) != 1)
+            errno = 0;
+            ping_data_size = (unsigned int)strtoul(optparse_state.optarg, &endptr, 10);
+            if (errno != 0 || optparse_state.optarg == endptr || *endptr != '\0')
                 usage(1);
             size_flag = 1;
             break;
@@ -830,12 +842,14 @@ int main(int argc, char **argv)
         case 'Q':
             verbose_flag = 0;
             quiet_flag = 1;
-            if (sscanf(optparse_state.optarg, "%f", &opt_value_float) != 1)
+            errno = 0;
+            opt_val_double = strtod(optparse_state.optarg, &endptr);
+            if (errno != 0 || optparse_state.optarg == endptr || (*endptr != '\0' && *endptr != ','))
                 usage(1);
-            if (opt_value_float < 0) {
+            if (opt_val_double < 0) {
                 usage(1);
             }
-            report_interval = opt_value_float * 1e9;
+            report_interval = opt_val_double * 1e9;
 
             /* recognize keyword(s) after number, ignore everything else */
             {
@@ -921,9 +935,10 @@ int main(int argc, char **argv)
 
 #if defined(DEBUG) || defined(_DEBUG)
         case 'z':
-            if (sscanf(optparse_state.optarg, "0x%x", &debugging) != 1)
-                if (sscanf(optparse_state.optarg, "%u", &debugging) != 1)
-                    usage(1);
+            errno = 0;
+            debugging = (unsigned int)strtoul(optparse_state.optarg, &endptr, 0);
+            if (errno != 0 || optparse_state.optarg == endptr || *endptr != '\0')
+                usage(1);
 
             break;
 #endif /* DEBUG || _DEBUG */