[monitoring-plugins] Merge commit from fork

[GitHub]

    Module: monitoring-plugins
    Branch: master
    Commit: 6d77e40f850f03b1d8fbbb48449eccdb9792618c
    Author: Holger Weiß <holger at zedat.fu-berlin.de>
 Committer: GitHub <noreply at github.com>
      Date: Wed Jul  1 13:05:15 2026 +0200
       URL: https://www.monitoring-plugins.org/repositories/monitoring-plugins/commit/?id=6d77e40f

Merge commit from fork

* check_icmp: Reject more than 65535 target hosts

The number of -H hosts is counted into an unsigned short, so supplying
more than 65535 hosts wraps the counter.  The subsequent calloc(3) then
allocates an undersized hosts array while the later parsing loop still
writes one entry per host, overflowing the heap buffer.  As
process_arguments() runs before we drop privileges via setuid(getuid()),
this happens while still running as root on setuid-root installs.

Guard both places where the counter is incremented and bail out with a
usage error once 65535 hosts are reached, rather than wrapping silently.

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

* 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 at ChristopherKreft.de>

* check_icmp: Reject negative ICMP data length

The -b/--size handler checks the lower bound after casting the value to
unsigned long while checking the upper bound as a signed comparison.  A
negative argument such as "-b -65536" therefore satisfies both checks.
The value is then truncated to an undersized icmp_data_size, which later
serves as the size of the ICMP send buffer, so building the packet
overflows that buffer.

Compare the size as a signed long against both bounds so negative values
are rejected.

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

* Update NEWS

---

 NEWS                      | 10 ++++++++++
 plugins-root/check_icmp.c | 33 +++++++++++++++++++++------------
 2 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/NEWS b/NEWS
index 87d44a56..13a2c0b5 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,15 @@
 This file documents the major additions and syntax changes between releases.
 
+3.0.1 1st July 2026
+	FIXES
+	* check_icmp
+	  * Reject more than 65535 target hosts to prevent a buffer overflow
+	    before privilege drop
+	  * Fix privileged out-of-bounds access when a single-character
+	    threshold is given
+	  * Reject negative ICMP data length (-b) to prevent a buffer overflow
+	    when building the ICMP packet
+
 3.0.0 16th Jun 2026
     Codename: Johann Georg Elser
 
diff --git a/plugins-root/check_icmp.c b/plugins-root/check_icmp.c
index 1390a03e..14e63c89 100644
--- a/plugins-root/check_icmp.c
+++ b/plugins-root/check_icmp.c
@@ -361,6 +361,9 @@ check_icmp_config_wrapper process_arguments(int argc, char **argv) {
 				enforced_ai_family = AF_INET6;
 				break;
 			case 'H': {
+				if (result.config.number_of_hosts == USHRT_MAX) {
+					usage_va("Number of specified hosts exceeds %u", USHRT_MAX);
+				}
 				result.config.number_of_hosts++;
 				break;
 			}
@@ -378,6 +381,9 @@ check_icmp_config_wrapper process_arguments(int argc, char **argv) {
 
 	char **tmp = &argv[optind];
 	while (*tmp) {
+		if (result.config.number_of_hosts == USHRT_MAX) {
+			usage_va("Number of specified hosts exceeds %u", USHRT_MAX);
+		}
 		result.config.number_of_hosts++;
 		tmp++;
 	}
@@ -405,7 +411,7 @@ check_icmp_config_wrapper process_arguments(int argc, char **argv) {
 			switch (arg) {
 			case 'b': {
 				long size = strtol(optarg, NULL, 0);
-				if ((unsigned long)size >= (sizeof(struct icmp) + sizeof(struct icmp_ping_data)) &&
+				if (size >= (long)(sizeof(struct icmp) + sizeof(struct icmp_ping_data)) &&
 					size < MAX_PING_DATA) {
 					result.config.icmp_data_size = (unsigned short)size;
 				} else {
@@ -1920,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);