check_icmp: Fix parsing of single-char threshold

The threshold parser starts a pointer at the last character of the
string and walks it backwards until it reaches the second character.
This assumes the string is at least two characters long.  For a
single-character threshold such as "-w 1" or "-c 1", the pointer
underflows past the start of the string and keeps dereferencing memory
out of bounds.

Beyond the out-of-bounds reads, an out-of-bounds write can occur if a
stray '%' or ',' byte happens to turn up while scanning backwards.  In
that case, the parser writes a NUL byte at that out-of-bounds address
(the ',' case additionally re-reads forward from there via strtoul(3)).

Only run the descending scan when the string has at least two
characters, leaving the behaviour for all valid thresholds unchanged.

Reported-by: Christopher Kreft <Email@ChristopherKreft.de>

1 files changed, 14 insertions, 11 deletions

diff --git a/plugins-root/check_icmp.c b/plugins-root/check_icmp.c
index f3c83ee0..8f5c1fe4 100644
--- a/plugins-root/check_icmp.c
+++ b/plugins-root/check_icmp.c
@@ -1926,18 +1926,21 @@ static get_threshold_wrapper get_threshold(char *str, check_icmp_threshold thres
         }
 
         /* pointer magic slims code by 10 lines. i is bof-stop on stupid libc's */
-        bool is_at_last_char = false;
-        char *tmp = &str[strlen(str) - 1];
-        while (tmp != &str[1]) {
-                if (*tmp == '%') {
-                        *tmp = '\0';
-                } else if (*tmp == ',' && is_at_last_char) {
-                        *tmp = '\0'; /* reset it so get_timevar(str) works nicely later */
-                        result.threshold.pl = (unsigned char)strtoul(tmp + 1, NULL, 0);
-                        break;
+        size_t len = strlen(str);
+        if (len >= 2) {
+                bool is_at_last_char = false;
+                char *tmp = &str[len - 1];
+                while (tmp != &str[1]) {
+                        if (*tmp == '%') {
+                                *tmp = '\0';
+                        } else if (*tmp == ',' && is_at_last_char) {
+                                *tmp = '\0'; /* reset it so get_timevar(str) works nicely later */
+                                result.threshold.pl = (unsigned char)strtoul(tmp + 1, NULL, 0);
+                                break;
+                        }
+                        is_at_last_char = true;
+                        tmp--;
                 }
-                is_at_last_char = true;
-                tmp--;
         }
 
         get_timevar_wrapper parsed_time = get_timevar(str);