[Nagiosplug-devel] Re: Major Secuirty Hole in Netsaint/Nagios.

[Ethan Galstad]

Tyler -

Thanks for the note.  I'm forwarding this to the nagios-devel and 
nagiosplug-devel lists, as I think this should be discussed there 
with others before making changes.

As far as patches are concerned, I won't be able to make any until at 
least next week.  My cable provider just did an "upgrade" which now 
puts me 18 hops away from the DHCP server - this caused the ISC DHCP 
client on my LRP box to croak (TTL is hardcoded to16 in the 
DHCPDISCOVER packet), so I've been fighting with trying to recompile 
a hacked version of the client that will actually run properly on my 
firewall.  To top that off, I just discovered that the power supply 
in my development box died sometime in the past few days and my case 
is a nice toasty 110 degrees....  So I have to wait until it cools 
down before trying to build a new box tomorrow or this weekend.  I 
love the timing of problems... :-)

--

Anyway, on to the issue at hand.  I had been thinking about stripping 
all single and double quotes out of the plugin output, so this might 
was well be the time to do it all.  This of course will break any 
plugin that returns a hyperlink in the output.

I would like to keep the following characters, available for output, 
as they are more benign without the help of others, especially is 
quoted properly in notification commands, etc:

(,),!,?

It was not mentioned, but % is also a potentially nasty character, 
but I still think it should stay (e.g. for packet loss percentages in 
check_ping, check_disk, etc).  It all comes down to a tradeoff 
between security and flexibility.  I believe that the five characters 
I mention should be okay, as long as notification commands, etc. 
contain quotes to prevent the shell from interpreting them.  That's 
more of a user education issue than anything else.  

BTW, although the message only mentioned stripping these chars from 
plugin output (and acknowledgements), I should probably also strip 
these from names of hosts, services, etc.  Anyone see a good reason 
not to strip these characters?



On 27 Jun 2002 at 17:31, Tyler Lund wrote:

> Hello,
> 
> I have found what I think to be a major security flaw in the current
> stable 0.0.7 release of Netsaint. From what I can tell, this flaw
> also exists in the current 1.0 beta 3 release of Nagios. 
> 
> Basically the macro_output string does not get checked for shell
> interpreted characters prior to being executed by popen() in my_sysem().
> 
> This was discovered when attempting to insert an apostrophie character
> into an ack command and noting that the character was interpreted by
> the shell. Further investigation revealed that ANY special shell
> character will be interpreted, including a backtick ` character. This
> will allow anyone acking an alarm to execute arbitrary commands on the
> server as the netsaint user. 
> 
> Extapolating, this macro is also used to store output from service
> checks, including strings passed to it from NRPE and other agents.
> Using this method, an attacker would be able to execute commands on the
> central netsaint server by modifying output from a monitored host. It
> would be pretty trivial to gain access to the central server depending
> on the permissions of the user under which netsaint is running.
> 
> Has this issue been addressd before? I've searched mailing lists and
> Changelogs to no avail. I'm sure this can be implimented
> in a cleaner fashion, but I've patched my 0.0.7 source files
> to sterilize macros for shell execution: 
> 
> char *sanitize_shell_string(char *str)
> /* takes string and escapes all metacharacters.  should be used before
>    including string in system() or similar call. */
> {
>   int i,j = 0;
>   char *new = malloc(sizeof(char) * (strlen(str) * 2 + 1));
>   for (i = 0; i < strlen(str); i++) {
>     switch (str[i]) {
>       case '|': case '&': case ';': case '(': case ')': case '<':
>       case '>': case '\'': case '"': case '*': case '?': case '\\':
>       case '[': case ']': case '$': case '!': case '#': case '`':
>       case '{': case '}':
>         new[j] = '\\';
>         j++;
>         break;
>       default:
>         break;
>     }
>     new[j] = str[i];
>     j++;
>   }
>   new[j] = '\0';
>   return new;
> }
> 
> and in process_macros():
> 
> else if(!strcmp(temp_buffer,"OUTPUT")) {
> 	char *clean = sanitize_shell_string(macro_output);
> 	strncat(output_buffer,(macro_output==NULL)?"":clean,buffer_length-strlen(clean)-1);
> }
> 
> ...repeat for other macros,  etc. 
> 
> If I'm way off on this for Nagios please let me know, but I would suggest
> that for at least Netsaint that a patch release be made for this
> extremely dangerous flaw. 
> 
> Thanks!
> 
> -- 
> Tyler Lund - Sr. Network Engineer, Springboard Hosting
> tlund at springboardhosting.com - www.springboardhosting.com
> ***      Thank you for contacting Springboard Support     ***
> ***   support at springboardhosting.com      919-852-0690    ***
> 



Ethan Galstad,
Nagios Developer
---
Email: nagios at nagios.org
Website: http://www.nagios.org