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

[Subhendu Ghosh]

I understand the issue but would like to see if it is possible to return a 
hyperlink in the output, at least in the case that the plugins are 
executed locally. Since there is no differentiation between local and 
remote that would be hard to currently. 

The other way would be to allow only the urlize plugin to pass a 
hyperlink.

The third option would be to add a field to the config that creates the 
hyperlink - i.e internalize the urlize plugin into nagios.

I have found it very useful for links to traceroute and other tools.

Other than the above comments, I think the five characters below should 
suffice.

About host names and service names - my preference has been to write them 
in following case sensitive DNS naming rules. So removing the characters 
from the names would not be a bad thing.

Question - should the plugins be forced to check / sanitize their output 
to limit themsleves to alphanumeric+5?
I would think - yes, but not to escape anything.

The escape should be done on the receiving end.


A couple of other things - I am not sure about as yet...

1. Does this have any impact on the check_command 

2. Does this have an impact on the maxsize of the input and output 
buffers



On Fri, 28 Jun 2002, Ethan Galstad wrote:

<SNIP>
> 
> 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
> ---

-sg
--