nrpe, arguments and security

[Peter Åstrand]

The nrpe SECURITY file, the dont_blame_nrpe parameter, the log messages
etc gives a clear message: enabling command arguments is extremely
dangerous. It doesn't say *why* this is dangerous, though. 

nrpe tries to filter out "nasty" meta characters, but it doesn't do this 
good enough. Several characters, such as # and ; are missing from the 
NASTY_METACHARS definition. This is a security hole. Assume a nrpe.cfg 
with:

dont_blame_nrpe=1
command[echo]=echo $ARG1$

In this case, arbitrary commands can be run, by running:

  check_nrpe -H targethost -c echo -a 'foo; myevilcommand'

or

  check_nrpe -H targethost -c echo -a 'foo;
myevilcommand'


In short: I'm not happy with the current implementation. It should be 
possible to provide a *safe* way of passing arguments to plugins. The 
attached patch limits the arguments to [A-Za-z0-9 ]. Can anyone find a 
security problem with an implementation like this?

(The main problem with the current implementation is that nrpe executes
programs through the popen() library call. It has been known for a long
time that this call is pretty unsafe. In this case, however, it's a bit
hard to get rid of it: We would have to split the string in nrpe.cfg into
an argument list. Limiting the allowed chacters is much simpler, and has
an additional advantage: The executed command/plugin may eventually pass
on it's arguments to a shell.)

I've looked at nrpe_nt as well. Since it does not execute the command 
through a shell, it should be much safer. Limiting the arguments to 
[A-Za-z0-9 ] is probably a good idea anyway. 

Comments?


Index: nrpe.c
===================================================================
RCS file: /cvsroot/nagios/nrpe/src/nrpe.c,v
retrieving revision 1.35
diff -u -r1.35 nrpe.c
--- nrpe.c	20 May 2004 22:41:02 -0000	1.35
+++ nrpe.c	30 Nov 2004 14:41:56 -0000
@@ -34,7 +34,7 @@
 
 #define DEFAULT_COMMAND_TIMEOUT	60			/* default timeout for execution of plugins */
 #define MAXFD                   64
-#define NASTY_METACHARS         "|`&><'\"\\[]{}"
+#define ALLOWED_ARGUMENT_CHARS  " !abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
 
 
 int process_arguments(int,char **);
@@ -50,7 +50,7 @@
 void free_memory(void);
 int is_an_allowed_host(char *);
 int validate_request(packet *);
-int contains_nasty_metachars(char *);
+int contains_illegal_chars(char *);
 int process_macros(char *,char *,int);
 int my_system(char *,int,int *,char *,int);            	/* executes a command via popen(), but also protects against timeouts */
 void my_system_sighandler(int);				/* handles timeouts when executing commands via my_system() */
@@ -1353,8 +1353,8 @@
 	        }
 
 	/* make sure request doesn't contain nasties */
-	if(contains_nasty_metachars(pkt->buffer)==TRUE){
-		syslog(LOG_ERR,"Error: Request contained illegal metachars!");
+	if(contains_illegal_chars(pkt->buffer)==TRUE){
+		syslog(LOG_ERR,"Error: Request contained illegal chars!");
 		return ERROR;
 	        }
 
@@ -1409,14 +1409,14 @@
 
 
 
-/* tests whether a buffer contains illegal metachars */
-int contains_nasty_metachars(char *str){
+/* tests whether a buffer contains illegal chars */
+int contains_illegal_chars(char *str){
 	int result;
 
 	if(str==NULL)
 		return FALSE;
 	
-	result=strcspn(str,NASTY_METACHARS);
+	result=strspn(str,ALLOWED_ARGUMENT_CHARS);
 	if(result!=strlen(str))
 		return TRUE;
 

-- 
Peter Åstrand		Chief Developer
Cendio			www.thinlinc.com
Teknikringen 3		www.cendio.se
583 30 Linköping        Phone: +46-13-21 46 00




-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now. 
http://productguide.itmanagersjournal.com/