Peter Åstrand wrote:
> 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?
>
>
I've already implemented whitelist argument chars in current NRPE (which
isn't publicly available, since I haven't gotten the PK authentication
to work properly). Thanks for participating though.
Comments below.
> 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"
>
! is not currently in the safe-by-default list, as it is treated
different depending on where your /bin/sh points to. ,.% are, though.
>
> 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;
>
--
Andreas Ericsson [email protected]
OP5 AB www.op5.se
Lead Developer & NRPE maintainer
This post was automatically imported from historical nagios-devel mailing list archives
Original poster: [email protected]