[Pkg-nagios-devel] Bug#714281: more info & patch

[Václav Ovsík]

Hi,
I rebuilt nagios-plugins with `nostrip' option, so I found the
place/places where the plugin fails.
To clarify things a bit - I'm preparing a new monitoring system and
access from its new IP address is not allowed everywhere yet. So some checks
fails now and some (check_nt in this case) segfaults.

I have a 35 core dumps now and it was dumped at two places:


18 times:
Core was generated by `/usr/lib/nagios/plugins/check_nt -H 192.168.71.23 -p 12489 -v MEMUSE -p 12489 -'.
...
(gdb) frame 2
#2  main (argc=13, argv=0x7fffad2b6458) at check_nt.c:278
278			mem_commitLimit=atof(strtok(recv_buffer,"&"));


17 times:
Core was generated by `/usr/lib/nagios/plugins/check_nt -H 192.168.71.23 -p 12489 -v SERVICESTATE -p 1'.
...
(gdb) frame 2
#2  main (argc=13, argv=0x7fff44d25368) at check_nt.c:268
268				return_code=atoi(strtok(recv_buffer,"&"));


A connection to agent didn't transfer the required data and recv_buffer is 
(gdb) print recv_buffer
$1 = '\000' <repeats 8191 times>

A call to strtok() returned NULL and atoi() segfaults on such argument.
It is a bad idea to call atoi() on strtok() returned value without testing.

Nagios plugins rebuilt with the patch attached works without a segfault
for one day now.

Cheers
-- 
Zito
-------------- next part --------------
#! /bin/sh /usr/share/dpatch/dpatch-run
## 10_check_nt_npe.dpatch by Vaclav Ovsik <vaclav.ovsik at i.cz>
##
## DP: Fixes some NULL pointer dereference in check_nt.

@DPATCH@
diff -urNad '--exclude=CVS' '--exclude=.svn' '--exclude=.git' '--exclude=.arch' '--exclude=.hg' '--exclude=_darcs' '--exclude=.bzr' nagios-plugins-1.4.16~/plugins/check_nt.c nagios-plugins-1.4.16/plugins/check_nt.c
--- nagios-plugins-1.4.16~/plugins/check_nt.c	2013-06-29 18:11:20.000000000 +0200
+++ nagios-plugins-1.4.16/plugins/check_nt.c	2013-06-29 18:22:52.000000000 +0200
@@ -94,6 +94,7 @@
 	char *description=NULL,*counter_unit = NULL;
 	char *minval = NULL, *maxval = NULL, *errcvt = NULL;
 	char *fds=NULL, *tds=NULL;
+	char *numstr;
 
 	double total_disk_space=0;
 	double free_disk_space=0;
@@ -265,7 +266,10 @@
 			asprintf(&send_buffer,"%s&%u&%s&%s", req_password,(vars_to_check==CHECK_SERVICESTATE)?5:6,
 							 (show_all==TRUE) ? "ShowAll" : "ShowFail",value_list);
 			fetch_data (server_address, server_port, send_buffer);
-			return_code=atoi(strtok(recv_buffer,"&"));
+			numstr = strtok(recv_buffer,"&");
+			if (numstr == NULL)
+				die(STATE_UNKNOWN, _("could not fetch information from server\n"));
+			return_code=atoi(numstr);
 			temp_string=strtok(NULL,"&");
 			output_message = strdup (temp_string);
 		}
@@ -275,8 +279,14 @@
 
 		asprintf(&send_buffer,"%s&7", req_password);
 		fetch_data (server_address, server_port, send_buffer);
-		mem_commitLimit=atof(strtok(recv_buffer,"&"));
-		mem_commitByte=atof(strtok(NULL,"&"));
+		numstr = strtok(recv_buffer,"&");
+		if (numstr == NULL)
+			die(STATE_UNKNOWN, _("could not fetch information from server\n"));
+		mem_commitLimit=atof(numstr);
+		numstr = strtok(NULL,"&");
+		if (numstr == NULL)
+			die(STATE_UNKNOWN, _("could not fetch information from server\n"));
+		mem_commitByte=atof(numstr);
 		percent_used_space = (mem_commitByte / mem_commitLimit) * 100;
 		warning_used_space = ((float)warning_value / 100) * mem_commitLimit;
 		critical_used_space = ((float)critical_value / 100) * mem_commitLimit;