bug fix for avail.cgi

Support forum for Nagios Core, Nagios Plugins, NCPA, NRPE, NSCA, NDOUtils and more. Engage with the community of users including those using the open source solutions.
Locked
eponymousalias
Posts: 17
Joined: Mon Mar 07, 2016 5:38 am

bug fix for avail.cgi

Post by eponymousalias »

We experienced a problem with avail.cgi on a 64-bit platform. The fix is provided in the attached avail.c.saved_stamp.patch file. It applies directly to Nagios 3.5.0 and Nagios 3.5.1, and still applies as well (but with some fuzz/offset) to Nagios 4.0.8 and Nagios 4.1.1. Which is to say, while we found the bug in Nagios 3.5.1, it is still present in Nagios 4.1.1.

There are two parts to the bug fix. One, saved_stamp is declared as int when it should really be a time_t. This won't show up as a practical effect until the year 2038, but it ought to be fixed anyway.

The other part is more immediately serious. Within the compute_subject_downtime_times routine, saved_stamp is assigned to some value derived from the incoming archived log data. Trouble is, in one branch of the code it is properly limited to never be less than start_time, while in the other branch immediately below that, the same correction is missing.

We found this problem by running into it at a customer site, using rather large archived data files. Thus I don't have an exact characterization of what input data triggers the fault, nor any simple test data I can post here. But a look at the affected code will show the obvious parallelism and the obvious applicability of the patch.

The result of the unfixed code is that some time intervals are being restricted to the time interval specified on the command line, while some time intervals are not. Then when it comes to computing certain values such as TIME_OK_UNSCHEDULED, which is calculated on the fly as temp_subject->time_ok - temp_subject->scheduled_time_ok, the subtraction yields a negative number. This is then output using %lu (reflecting the use of unsigned 64-bit integers), resulting in a huge positive value such as 18446744073709465216 (which is equal to (2^^64) - 86400). This mistake can cause a lot of downstream confusion, depending on what is done with the output of the avail.cgi program.
Attachments
avail.c.saved_stamp.patch
Patch to consistently restrict time intervals in data processed by avail.cgi.
(627 Bytes) Downloaded 395 times
tmcdonald
Posts: 9117
Joined: Mon Sep 23, 2013 8:40 am

Re: bug fix for avail.cgi

Post by tmcdonald »

Much appreciated! I've brought this to the attention of our Core dev and he'll be taking a look into it.

For future reference, if you want to get this sort of thing on his radar immediately you can file it as an Issue on our Github: https://github.com/NagiosEnterprises/nagioscore

Github Issue for reference: https://github.com/NagiosEnterprises/na ... issues/114
Former Nagios employee
jfrickson

Re: bug fix for avail.cgi

Post by jfrickson »

The int vs time_t is definitely something we should fix. But the check for if(saved_stamp < start_time) fix has been in place since March 21, 2006 in version 3.0.4 via commit https://github.com/NagiosEnterprises/na ... abc8106dab

Or am I missing something?
eponymousalias
Posts: 17
Joined: Mon Mar 07, 2016 5:38 am

Re: bug fix for avail.cgi

Post by eponymousalias »

jfrickson wrote:The int vs time_t is definitely something we should fix. But the check for if(saved_stamp < start_time) fix has been in place since March 21, 2006 in version 3.0.4 via commit https://github.com/NagiosEnterprises/na ... abc8106dab

Or am I missing something?
Yes, you're missing something. I'm going to refer to the line numbers in the Nagios 4.2.1 source code for the cgi/avail.c file.

It's true that there is a similar correction at lines 2280 through 2282, after the assignment of saved_stamp at line 2278. But what is needed now is the same correction immediately after the assignment of saved_stamp at line 2304.
jfrickson

Re: bug fix for avail.cgi

Post by jfrickson »

eponymousalias wrote:It's true that there is a similar correction at lines 2280 through 2282, after the assignment of saved_stamp at line 2278. But what is needed now is the same correction immediately after the assignment of saved_stamp at line 2304.
Aha! Now I understand. I'll get the fix in for the next release. Thanks!
Locked