From 986b2479465648c49a7eefc3fbf4df8860e3e4b7 Mon Sep 17 00:00:00 2001 From: ghciv6 Date: Mon, 20 Dec 2021 22:39:57 +0000 Subject: - delay set_source_ip() until address_family is detected - add a test to check '-s' --- plugins-root/check_icmp.c | 5 ++++- plugins-root/t/check_icmp.t | 8 +++++++- 2 files changed, 11 insertions(+), 2 deletions(-) (limited to 'plugins-root') diff --git a/plugins-root/check_icmp.c b/plugins-root/check_icmp.c index 01ae174a..f97b0ed7 100644 --- a/plugins-root/check_icmp.c +++ b/plugins-root/check_icmp.c @@ -410,6 +410,7 @@ main(int argc, char **argv) #ifdef SO_TIMESTAMP int on = 1; #endif + char *source_ip = NULL; char * opts_str = "vhVw:c:n:p:t:H:s:i:b:I:l:m:64"; setlocale (LC_ALL, ""); @@ -542,7 +543,7 @@ main(int argc, char **argv) } break; case 's': /* specify source IP address */ - set_source_ip(optarg); + source_ip = optarg; break; case 'V': /* version */ print_revision (progname, NP_VERSION); @@ -597,6 +598,8 @@ main(int argc, char **argv) sockets |= HAVE_ICMP; else icmp_sockerrno = errno; + if( source_ip ) + set_source_ip(source_ip); #ifdef SO_TIMESTAMP if(setsockopt(icmp_sock, SOL_SOCKET, SO_TIMESTAMP, &on, sizeof(on))) diff --git a/plugins-root/t/check_icmp.t b/plugins-root/t/check_icmp.t index e043d4ed..55edc31b 100644 --- a/plugins-root/t/check_icmp.t +++ b/plugins-root/t/check_icmp.t @@ -12,7 +12,7 @@ my $allow_sudo = getTestParameter( "NP_ALLOW_SUDO", "no" ); if ($allow_sudo eq "yes" or $> == 0) { - plan tests => 16; + plan tests => 18; } else { plan skip_all => "Need sudo to test check_icmp"; } @@ -83,3 +83,9 @@ $res = NPTest->testCmd( is( $res->return_code, 2, "One of two host nonresponsive - two required" ); like( $res->output, $failureOutput, "Output OK" ); +$res = NPTest->testCmd( + "$sudo ./check_icmp -H $host_responsive -s 127.0.15.15 -w 10000ms,100% -c 10000ms,100% -n 1 -m 2" + ); +is( $res->return_code, 0, "IPv4 source_ip accepted" ); +like( $res->output, $successOutput, "Output OK" ); + -- cgit v1.2.3-74-g34f1 From 605405557102c04e740fc3249675cc5154436d11 Mon Sep 17 00:00:00 2001 From: Lorenz <12514511+RincewindsHat@users.noreply.github.com> Date: Tue, 15 Mar 2022 22:00:55 +0100 Subject: check_icmp: buffer offerflow (#1733) * Fix different overflows * Less includes * Add testcases * Remove unused variable * Remove unused and commented includes --- plugins-root/check_icmp.c | 66 +++++++++++++++++++++++++-------------------- plugins-root/t/check_icmp.t | 7 ++++- 2 files changed, 43 insertions(+), 30 deletions(-) (limited to 'plugins-root') diff --git a/plugins-root/check_icmp.c b/plugins-root/check_icmp.c index f97b0ed7..61198237 100644 --- a/plugins-root/check_icmp.c +++ b/plugins-root/check_icmp.c @@ -50,19 +50,11 @@ const char *email = "devel@monitoring-plugins.org"; #if HAVE_SYS_SOCKIO_H #include #endif -#include + #include -#include -#include -#include -#include -#include -#include #include -#include +#include #include -#include -#include #include #include #include @@ -71,8 +63,6 @@ const char *email = "devel@monitoring-plugins.org"; #include #include #include -#include -#include /** sometimes undefined system macros (quite a few, actually) **/ @@ -207,7 +197,7 @@ static int add_target(char *); static int add_target_ip(char *, struct sockaddr_storage *); static int handle_random_icmp(unsigned char *, struct sockaddr_storage *); static void parse_address(struct sockaddr_storage *, char *, int); -static unsigned short icmp_checksum(unsigned short *, int); +static unsigned short icmp_checksum(uint16_t *, size_t); static void finish(int); static void crash(const char *, ...); @@ -465,7 +455,6 @@ main(int argc, char **argv) /* Parse protocol arguments first */ for(i = 1; i < argc; i++) { while((arg = getopt(argc, argv, opts_str)) != EOF) { - unsigned short size; switch(arg) { case '4': if (address_family != -1) @@ -488,10 +477,10 @@ main(int argc, char **argv) /* Reset argument scanning */ optind = 1; + unsigned short size; /* parse the arguments */ for(i = 1; i < argc; i++) { while((arg = getopt(argc, argv, opts_str)) != EOF) { - unsigned short size; switch(arg) { case 'v': debug++; @@ -720,7 +709,7 @@ main(int argc, char **argv) static void run_checks() { - u_int i, t, result; + u_int i, t; u_int final_wait, time_passed; /* this loop might actually violate the pkt_interval or target_interval @@ -738,9 +727,9 @@ run_checks() /* we're still in the game, so send next packet */ (void)send_icmp_ping(icmp_sock, table[t]); - result = wait_for_reply(icmp_sock, target_interval); + wait_for_reply(icmp_sock, target_interval); } - result = wait_for_reply(icmp_sock, pkt_interval * targets); + wait_for_reply(icmp_sock, pkt_interval * targets); } if(icmp_pkts_en_route && targets_alive) { @@ -760,7 +749,7 @@ run_checks() * haven't yet */ if(debug) printf("Waiting for %u micro-seconds (%0.3f msecs)\n", final_wait, (float)final_wait / 1000); - result = wait_for_reply(icmp_sock, final_wait); + wait_for_reply(icmp_sock, final_wait); } } @@ -779,7 +768,7 @@ static int wait_for_reply(int sock, u_int t) { int n, hlen; - static unsigned char buf[4096]; + static unsigned char buf[65536]; struct sockaddr_storage resp_addr; union ip_hdr *ip; union icmp_packet packet; @@ -916,9 +905,27 @@ wait_for_reply(int sock, u_int t) if(debug) { char address[INET6_ADDRSTRLEN]; parse_address(&resp_addr, address, sizeof(address)); - printf("%0.3f ms rtt from %s, outgoing ttl: %u, incoming ttl: %u, max: %0.3f, min: %0.3f\n", - (float)tdiff / 1000, address, - ttl, ip->ip.ip_ttl, (float)host->rtmax / 1000, (float)host->rtmin / 1000); + + switch(address_family) { + case AF_INET: { + printf("%0.3f ms rtt from %s, outgoing ttl: %u, incoming ttl: %u, max: %0.3f, min: %0.3f\n", + (float)tdiff / 1000, + address, + ttl, + ip->ip.ip_ttl, + (float)host->rtmax / 1000, + (float)host->rtmin / 1000); + break; + }; + case AF_INET6: { + printf("%0.3f ms rtt from %s, outgoing ttl: %u, max: %0.3f, min: %0.3f\n", + (float)tdiff / 1000, + address, + ttl, + (float)host->rtmax / 1000, + (float)host->rtmin / 1000); + }; + } } /* if we're in hostcheck mode, exit with limited printouts */ @@ -980,7 +987,7 @@ send_icmp_ping(int sock, struct rta_host *host) icp->icmp_cksum = 0; icp->icmp_id = htons(pid); icp->icmp_seq = htons(host->id++); - icp->icmp_cksum = icmp_checksum((unsigned short*)buf, icmp_pkt_size); + icp->icmp_cksum = icmp_checksum((uint16_t*)buf, (size_t)icmp_pkt_size); if (debug > 2) printf("Sending ICMP echo-request of len %lu, id %u, seq %u, cksum 0x%X to host %s\n", @@ -1517,18 +1524,19 @@ get_threshold(char *str, threshold *th) } unsigned short -icmp_checksum(unsigned short *p, int n) +icmp_checksum(uint16_t *p, size_t n) { unsigned short cksum; long sum = 0; - while(n > 2) { - sum += *p++; - n -= sizeof(unsigned short); + /* sizeof(uint16_t) == 2 */ + while(n >= 2) { + sum += *(p++); + n -= 2; } /* mop up the occasional odd byte */ - if(n == 1) sum += (unsigned char)*p; + if(n == 1) sum += *((uint8_t *)p -1); sum = (sum >> 16) + (sum & 0xffff); /* add hi 16 to low 16 */ sum += (sum >> 16); /* add carry */ diff --git a/plugins-root/t/check_icmp.t b/plugins-root/t/check_icmp.t index 55edc31b..f6aa6813 100644 --- a/plugins-root/t/check_icmp.t +++ b/plugins-root/t/check_icmp.t @@ -12,7 +12,7 @@ my $allow_sudo = getTestParameter( "NP_ALLOW_SUDO", "no" ); if ($allow_sudo eq "yes" or $> == 0) { - plan tests => 18; + plan tests => 20; } else { plan skip_all => "Need sudo to test check_icmp"; } @@ -89,3 +89,8 @@ $res = NPTest->testCmd( is( $res->return_code, 0, "IPv4 source_ip accepted" ); like( $res->output, $successOutput, "Output OK" ); +$res = NPTest->testCmd( + "$sudo ./check_icmp -H $host_responsive -b 65507" + ); +is( $res->return_code, 0, "Try max paket size" ); +like( $res->output, $successOutput, "Output OK - Didn't overflow" ); -- cgit v1.2.3-74-g34f1 From ee50ddf6988e9d14502ed3fa4645dcd679f347f8 Mon Sep 17 00:00:00 2001 From: eriksejr Date: Thu, 14 Jul 2022 04:25:51 -0400 Subject: Set msg_namelen to the size of the sockaddr struct for the appropriate address family and not sockaddr_storage (#1771) Co-authored-by: Erik Sejr Co-authored-by: Lorenz <12514511+RincewindsHat@users.noreply.github.com> --- plugins-root/check_icmp.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'plugins-root') diff --git a/plugins-root/check_icmp.c b/plugins-root/check_icmp.c index 61198237..f8f15351 100644 --- a/plugins-root/check_icmp.c +++ b/plugins-root/check_icmp.c @@ -213,7 +213,7 @@ static int mode, protocols, sockets, debug = 0, timeout = 10; static unsigned short icmp_data_size = DEFAULT_PING_DATA_SIZE; static unsigned short icmp_pkt_size = DEFAULT_PING_DATA_SIZE + ICMP_MINLEN; -static unsigned int icmp_sent = 0, icmp_recv = 0, icmp_lost = 0; +static unsigned int icmp_sent = 0, icmp_recv = 0, icmp_lost = 0, ttl = 0; #define icmp_pkts_en_route (icmp_sent - (icmp_recv + icmp_lost)) static unsigned short targets_down = 0, targets = 0, packets = 0; #define targets_alive (targets - targets_down) @@ -223,7 +223,6 @@ static pid_t pid; static struct timezone tz; static struct timeval prog_start; static unsigned long long max_completion_time = 0; -static unsigned char ttl = 0; /* outgoing ttl */ static unsigned int warn_down = 1, crit_down = 1; /* host down threshold values */ static int min_hosts_alive = -1; float pkt_backoff_factor = 1.5; @@ -520,7 +519,7 @@ main(int argc, char **argv) add_target(optarg); break; case 'l': - ttl = (unsigned char)strtoul(optarg, NULL, 0); + ttl = (int)strtoul(optarg, NULL, 0); break; case 'm': min_hosts_alive = (int)strtoul(optarg, NULL, 0); @@ -948,6 +947,7 @@ static int send_icmp_ping(int sock, struct rta_host *host) { long int len; + size_t addrlen; struct icmp_ping_data data; struct msghdr hdr; struct iovec iov; @@ -979,6 +979,7 @@ send_icmp_ping(int sock, struct rta_host *host) if (address_family == AF_INET) { struct icmp *icp = (struct icmp*)buf; + addrlen = sizeof(struct sockaddr_in); memcpy(&icp->icmp_data, &data, sizeof(data)); @@ -995,7 +996,10 @@ send_icmp_ping(int sock, struct rta_host *host) } else { struct icmp6_hdr *icp6 = (struct icmp6_hdr*)buf; + addrlen = sizeof(struct sockaddr_in6); + memcpy(&icp6->icmp6_dataun.icmp6_un_data8[4], &data, sizeof(data)); + icp6->icmp6_type = ICMP6_ECHO_REQUEST; icp6->icmp6_code = 0; icp6->icmp6_cksum = 0; @@ -1016,7 +1020,7 @@ send_icmp_ping(int sock, struct rta_host *host) memset(&hdr, 0, sizeof(hdr)); hdr.msg_name = (struct sockaddr *)&host->saddr_in; - hdr.msg_namelen = sizeof(struct sockaddr_storage); + hdr.msg_namelen = addrlen; hdr.msg_iov = &iov; hdr.msg_iovlen = 1; -- cgit v1.2.3-74-g34f1