status.cgi segfault without hostgroup parameter on solaris 10

[Ton Voon]

On 4 Oct 2010, at 07:41, Michael Friedrich wrote:

> Ethan Galstad wrote:
>> Michael Friedrich wrote:
>>
>>> Since it's rather trivial to fix, I decided to create a patch for  
>>> Nagios
>>> too. Attached GIT diff against latest HEAD resolves those issues,
>>> changing behavior to 'all' when no parameters are provided.

Just a small note: the git diff you provided used different tab  
spacings, which makes it a bit more awkward to apply. Could you use  
the same syntax when providing a diff?

Andreas, Ethan: we discussed some syntax cleansing tool. I use  
perltidy for Opsview (primarily perl code) and it helps all developers  
to adhere to the same coding standards. Is there an equivalent in the  
C world?


>> Its probably better to patch url_encode().  I'm committing a simple
>> patch that most likely fixes the problem.  Can you verify if it fixes
>> things on Solaris?

While this fixes the segfault, it doesn't fix the issue of  
automatically setting host=all if nothing is specified.

I've added a new test t/610cgistatus.t which caught the host=all issue  
and I've applied Michael's patch to set =all if the value is NULL. The  
test only covers the one scenario though, rather than all the parts  
changed by Michael.

> Building the latest HEAD straight with
>
> $ git pull
> $ make distclean
> $ ./configure
> $ make cgis
>
> throws
>
> cgiutils.c: In function `url_encode':
> cgiutils.c:1371: warning: passing arg 1 of `strcpy' from incompatible
> pointer type
>
> because strcpy awaits char* instead of char[]

I've fixed this too in CVS. I'm not sure if leaving this in will cause  
a bug somewhere else to happen though (overwriting the index locations  
with 0?).

>
> Running in gdb works without sigsegv. But I would mark that as hack,
> which might cause problems for other use cases.

It's not a hack. It prevents a NULL from being returned from  
url_encode - that's valid.

> On the webpage, Nagios 3.2.3 is already out. I don't really  
> understand - now there's a "possible fix for solaris segfault" and  
> this is put straight forward into a new release? I thought advanced  
> testing would be needed, also having something in mind with testing  
> on the 3 of you.

Again, I think that's valid. Ethan did some testing when applying  
url_encode, but didn't setup an entire Solaris environment to test the  
specific scenario. But this particular problem can be seen with the  
host=(null) (as Linux will not segfault but instead return this string  
instead), so testing for the removal of (null) will prove that this  
bug has been removed.

Ton


------------------------------------------------------------------------------
Nokia and AT&T present the 2010 Calling All Innovators-North America contest
Create new apps & games for the Nokia N8 for consumers in  U.S. and Canada
$10 million total in prizes - $4M cash, 500 devices, nearly $6M in marketing
Develop with Nokia Qt SDK, Web Runtime, or Java and Publish to Ovi Store 
http://p.sf.net/sfu/nokia-dev2dev