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

[Ethan Galstad]

I just posted updated code to CVS today to take a stab at fixing 
this.  I added an "illegal_macro_output_chars" variable to the main 
config file that allow the user what characters should be stripped 
from the $OUTPUT and $PERFDATA macros before they are substituted.  
Currently the nasty chars (that the user can specify) are stripped 
out completely - no escaping is done of anything that remains int he 
macros, although we could think about doing this as well.

I decided to only strip the macros before they're substituted into 
notification commands, performance data commands, event handlers, 
etc.  No characters are stripped from the internal buffers that 
actually hold the plugin output, as I want to retain this.  I don't 
see a problem with keeping these characters internally or writing 
them out to the status log (as they are never used in a command).

On 28 Jun 2002 at 10:59, Subhendu Ghosh wrote:

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

Do you use the links from the web or wap interface?  Or from the 
notification messages you receive.  The way I wrote things, quotes 
would be stripped from the output macro for notifications, but would 
remain in the status data for viewing via the web interface.  So that 
won't break hyperlinks for most users.

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

I added an "illegal_object_name_chars" variable to the main config 
file.  Instead of enforcing what is and isn't legal, I decided to let 
the user decide.  Single and double quotes in object names can cause 
web interface issues though, so its best to not allow them in object 
names.

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

Well, we could... but in the end its not very effective, since its 
easy to write custom plugins that break the rules.  The sanitization 
should happen within Nagios.

> 
> The escape should be done on the receiving end.

I was thinking about this and I don't know if we actually need to try 
and escape anything.  One problem is that I don't know how the user's 
commands are defined and how they'll handle escaped characters.  The 
other thing is that if you strip out most nasty characters (backtick, 
etc.), you should be okay with leaving more benign characters like 
()?! unescaped.  This assumes that the user is enclosing the macros 
in quotes in their command definitions.

> 
> 
> A couple of other things - I am not sure about as yet...
> 
> 1. Does this have any impact on the check_command 

Nope - only the $OUTPUT and $PERFDATA macros are sanitizes before the 
are sent to notification commands, performance data, event handlers, 
etc.

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

No again - the way I did things, nothing gets done until the macros 
are just about to be substituted into command lines that are going to 
be executed.

> 
> 
> 
> 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
> -- 
> 
> 
> 
> 
> -------------------------------------------------------
> This sf.net email is sponsored by:ThinkGeek
> Caffeinated soap. No kidding.
> http://thinkgeek.com/sf
> _______________________________________________
> Nagiosplug-devel mailing list
> Nagiosplug-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nagiosplug-devel
> 



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