Variables encoded twice

[Bernd Arnold]

Hi,

the patch from Armin is against 1.50 (current version in CVS).

The url_encode(...) calls were added in version 1.49, see http://nagios.cvs.sourceforge.net/nagios/nagios/cgi/avail.c?r1=1.48&r2=1.49.

1.49 is titled "XSS vulnerability fixes".

I don't know the depth of security, but unencoded strings can change the behavior if something like special characters (' < > &) are submitted. I think it's important that such things are encrypted, though I also think it's sufficient when a function like PHP's htmlentities function is used (keeps spaces as spaces, not +). Maybe host_name for example cannot contain such characters, but it's safer to encode everything.

http://de.php.net/manual/de/function.htmlentities.php (sorry for this comparison).

So better replace url_encode(...) by html_encode(...)?

Regards
Bernd


-------- Original-Nachricht --------
> Datum: Thu, 31 Jul 2008 19:06:51 +0100
> Von: Andy Shellam <andy.shellam-lists at mailnetwork.co.uk>
> An: nagios-devel at lists.sourceforge.net
> Betreff: Re: [Nagios-devel] Variables encoded twice

> Hi Armin,
> 
> What Nagios version is this patch against?  I only ask because I 
> submitted a very similar patch (albeit in a non-diff format) which 
> appears to have made it into 3.0.3.
> 
> Thanks,
> 
> Andy
> 
> Armin Wolfermann wrote:
> > Hi all,
> >
> > in several CGIs the values of hidden form variables are encoded while
> > this will be done again automatically on form submission. The patch
> > against current CVS:
> >
> > Index: avail.c
> > ===================================================================
> > RCS file: /cvsroot/nagios/nagios/cgi/avail.c,v
> > retrieving revision 1.50
> > diff -u -r1.50 avail.c
> > --- avail.c	23 Jun 2008 20:47:42 -0000	1.50
> > +++ avail.c	31 Jul 2008 14:23:09 -0000
> > @@ -512,11 +512,11 @@
> >  			if(display_type==DISPLAY_HOSTGROUP_AVAIL)
> >  				printf("<input type='hidden' name='hostgroup'
> value='%s'>\n",hostgroup_name);
> >  			if(display_type==DISPLAY_HOST_AVAIL ||
> display_type==DISPLAY_SERVICE_AVAIL)
> > -				printf("<input type='hidden' name='host'
> value='%s'>\n",url_encode(host_name));
> > +				printf("<input type='hidden' name='host' value='%s'>\n",host_name);
> >  			if(display_type==DISPLAY_SERVICE_AVAIL)
> >  				printf("<input type='hidden' name='service'
> value='%s'>\n",svc_description);
> >  			if(display_type==DISPLAY_SERVICEGROUP_AVAIL)
> > -				printf("<input type='hidden' name='servicegroup'
> value='%s'>\n",url_encode(servicegroup_name));
> > +				printf("<input type='hidden' name='servicegroup'
> value='%s'>\n",servicegroup_name);
> >  
> >  			printf("<input type='hidden' name='assumeinitialstates'
> value='%s'>\n",(assume_initial_states==TRUE)?"yes":"no");
> >  			printf("<input type='hidden' name='assumestateretention'
> value='%s'>\n",(assume_state_retention==TRUE)?"yes":"no");
> > Index: histogram.c
> > ===================================================================
> > RCS file: /cvsroot/nagios/nagios/cgi/histogram.c,v
> > retrieving revision 1.27
> > diff -u -r1.27 histogram.c
> > --- histogram.c	19 May 2008 18:42:26 -0000	1.27
> > +++ histogram.c	31 Jul 2008 14:23:10 -0000
> > @@ -407,9 +407,9 @@
> >  			printf("<form method=\"GET\" action=\"%s\">\n",HISTOGRAM_CGI);
> >  			printf("<input type='hidden' name='t1' value='%lu'>\n",(unsigned
> long)t1);
> >  			printf("<input type='hidden' name='t2' value='%lu'>\n",(unsigned
> long)t2);
> > -			printf("<input type='hidden' name='host'
> value='%s'>\n",url_encode(host_name));
> > +			printf("<input type='hidden' name='host' value='%s'>\n",host_name);
> >  			if(display_type==DISPLAY_SERVICE_HISTOGRAM)
> > -				printf("<input type='hidden' name='service'
> value='%s'>\n",url_encode(svc_description));
> > +				printf("<input type='hidden' name='service'
> value='%s'>\n",svc_description);
> >  
> >  
> >  			printf("<tr><td CLASS='optBoxItem' valign=top align=left>Report
> period:</td><td CLASS='optBoxItem' valign=top align=left>Assume state
> retention:</td></tr>\n");
> > @@ -789,9 +789,9 @@
> >  
> >  			printf("<TABLE BORDER=0 cellpadding=5>\n");
> >  			printf("<form method=\"GET\" action=\"%s\">\n",HISTOGRAM_CGI);
> > -			printf("<input type='hidden' name='host'
> value='%s'>\n",url_encode(host_name));
> > +			printf("<input type='hidden' name='host' value='%s'>\n",host_name);
> >  			if(display_type==DISPLAY_SERVICE_HISTOGRAM)
> > -				printf("<input type='hidden' name='service'
> value='%s'>\n",url_encode(svc_description));
> > +				printf("<input type='hidden' name='service'
> value='%s'>\n",svc_description);
> >  
> >  			printf("<tr><td class='reportSelectSubTitle' align=right>Report
> Period:</td>\n");
> >  			printf("<td class='reportSelectItem'>\n");
> > Index: history.c
> > ===================================================================
> > RCS file: /cvsroot/nagios/nagios/cgi/history.c,v
> > retrieving revision 1.31
> > diff -u -r1.31 history.c
> > --- history.c	23 Jun 2008 20:47:44 -0000	1.31
> > +++ history.c	31 Jul 2008 14:23:10 -0000
> > @@ -204,9 +204,9 @@
> >  
> >  		printf("<table border=0 CLASS='optBox'>\n");
> >  		printf("<form method=\"GET\" action=\"%s\">\n",HISTORY_CGI);
> > -		printf("<input type='hidden' name='host'
> value='%s'>\n",(show_all_hosts==TRUE)?"all":url_encode(host_name));
> > +		printf("<input type='hidden' name='host'
> value='%s'>\n",(show_all_hosts==TRUE)?"all":host_name);
> >  		if(display_type==DISPLAY_SERVICES)
> > -			printf("<input type='hidden' name='service'
> value='%s'>\n",url_encode(svc_description));
> > +			printf("<input type='hidden' name='service'
> value='%s'>\n",svc_description);
> >  		printf("<input type='hidden' name='archive'
> value='%d'>\n",log_archive);
> >  
> >  		printf("<tr>\n");
> > Index: notifications.c
> > ===================================================================
> > RCS file: /cvsroot/nagios/nagios/cgi/notifications.c,v
> > retrieving revision 1.25
> > diff -u -r1.25 notifications.c
> > --- notifications.c	19 May 2008 18:42:27 -0000	1.25
> > +++ notifications.c	31 Jul 2008 14:23:10 -0000
> > @@ -212,11 +212,11 @@
> >  		printf("<table border=0 CLASS='optBox'>\n");
> >  		printf("<form method='GET' action='%s'>\n",NOTIFICATIONS_CGI);
> >  		if(query_type==FIND_SERVICE){
> > -			printf("<input type='hidden' name='host'
> value='%s'>\n",url_encode(query_host_name));
> > -			printf("<input type='hidden' name='service'
> value='%s'>\n",url_encode(query_svc_description));
> > +			printf("<input type='hidden' name='host'
> value='%s'>\n",query_host_name);
> > +			printf("<input type='hidden' name='service'
> value='%s'>\n",query_svc_description);
> >  	                }
> >  		else
> > -			printf("<input type='hidden' name='%s'
> value='%s'>\n",(query_type==FIND_HOST)?"host":"contact",url_encode((query_type==FIND_HOST)?query_host_name:query_contact_name));
> > +			printf("<input type='hidden' name='%s'
> value='%s'>\n",(query_type==FIND_HOST)?"host":"contact",(query_type==FIND_HOST)?query_host_name:query_contact_name);
> >  		printf("<input type='hidden' name='archive'
> value='%d'>\n",log_archive);
> >  		printf("<tr>\n");
> >  		if(query_type==FIND_SERVICE)
> > Index: statusmap.c
> > ===================================================================
> > RCS file: /cvsroot/nagios/nagios/cgi/statusmap.c,v
> > retrieving revision 1.40
> > diff -u -r1.40 statusmap.c
> > --- statusmap.c	19 May 2008 18:42:28 -0000	1.40
> > +++ statusmap.c	31 Jul 2008 14:23:13 -0000
> > @@ -700,7 +700,7 @@
> >  		printf("<form method=\"POST\" action=\"%s\">\n",STATUSMAP_CGI);
> >  		printf("<table border=0 CLASS='optBox'>\n");
> >  		printf("<tr><td valign=top>\n");
> > -		printf("<input type='hidden' name='host'
> value='%s'>\n",url_encode(host_name));
> > +		printf("<input type='hidden' name='host' value='%s'>\n",host_name);
> >  		printf("<input type='hidden' name='layout'
> value='%d'>\n",layout_method);
> >  
> >  		printf("</td><td valign=top>\n");
> > Index: trends.c
> > ===================================================================
> > RCS file: /cvsroot/nagios/nagios/cgi/trends.c,v
> > retrieving revision 1.41
> > diff -u -r1.41 trends.c
> > --- trends.c	23 Jun 2008 20:47:46 -0000	1.41
> > +++ trends.c	31 Jul 2008 14:23:15 -0000
> > @@ -454,9 +454,9 @@
> >  				printf("<input type='hidden' name='nomap' value=''>\n");
> >  			printf("<input type='hidden' name='t1' value='%lu'>\n",(unsigned
> long)t1);
> >  			printf("<input type='hidden' name='t2' value='%lu'>\n",(unsigned
> long)t2);
> > -			printf("<input type='hidden' name='host'
> value='%s'>\n",url_encode(host_name));
> > +			printf("<input type='hidden' name='host' value='%s'>\n",host_name);
> >  			if(display_type==DISPLAY_SERVICE_TRENDS)
> > -				printf("<input type='hidden' name='service'
> value='%s'>\n",url_encode(svc_description));
> > +				printf("<input type='hidden' name='service'
> value='%s'>\n",svc_description);
> >  
> >  			printf("<input type='hidden' name='assumeinitialstates'
> value='%s'>\n",(assume_initial_states==TRUE)?"yes":"no");
> >  			printf("<input type='hidden' name='assumestateretention'
> value='%s'>\n",(assume_state_retention==TRUE)?"yes":"no");
> >
> >
> > Regards,
> > Armin Wolfermann
> > OSN Online Service Nuremberg
> >
> >
> -------------------------------------------------------------------------
> > This SF.Net email is sponsored by the Moblin Your Move Developer's
> challenge
> > Build the coolest Linux based applications with Moblin SDK & win great
> prizes
> > Grand prize is a trip for two to an Open Source event anywhere in the
> world
> > http://moblin-contest.org/redirect.php?banner_id=100&url=/
> > _______________________________________________
> > Nagios-devel mailing list
> > Nagios-devel at lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/nagios-devel
> >   
> 
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's
> challenge
> Build the coolest Linux based applications with Moblin SDK & win great
> prizes
> Grand prize is a trip for two to an Open Source event anywhere in the
> world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Nagios-devel mailing list
> Nagios-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nagios-devel

-- 
Der GMX SmartSurfer hilft bis zu 70% Ihrer Onlinekosten zu sparen! 
Ideal für Modem und ISDN: http://www.gmx.net/de/go/smartsurfer

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/