mon.cgi: patch to untaint monitor output
Ed Ravin <eravin <at> panix.com>
2007-07-12 14:53:56 GMT
One of my custom monitors was printing the output of a syslog entry
as its summary output. The syslog entry was from a mail program,
so it had stuff like "to=<whatever <at> example.com>". But in mon.cgi's
output in my Web browser, it just said "to=".
This is because mon.cgi is just dropping the output into a web page,
where the browser parses it for HTML. Years ago we added a call
to HTML::Entities:encode_entities to mon.cgi so that messages typed
in with the ACK command would not get confused with HTML - the attached
patch extends that functionality to monitor output.
There are security implications here - if an outside party could get
control of the output of a Mon script (easy in my case, since the data
comes from syslog and includes error messages from a remote host), that
outside party could cobble together some HTML that eventually gets
executed in the Mon user's browser (i.e. cross-site scripting).
The attached patch renames the "untaint_ack_msgs" parameter to
"untaint_all_msgs" and, when set, not only untaints ACK messages,
but also untaints last_summary and last_detail output before displaying
it.
I recommend that we remove this parameter completely and always untaint
messages before displaying them in the CGI interface - it's the right
thing to do. The cost is one more Perl module dependency, but there
are already a host of Perl modules needed to run mon and one more is
not going to hurt much.
I haven't worked on this code for several years so I may have missed
something - an extra pair of eyes on this patch would be appreciated.
-- Ed
--- mon.cgi.pl 2005-04-21 16:56:29.000000000 -0400
+++ mon2.cgi.pl 2007-07-12 10:32:59.000000000 -0400
<at> <at> -143,7 +143,7 <at> <at>
$monhost_and_port_args $monhost_and_port_args_meta
$has_read_config $moncgi_config_file $cf_file_mtime
$mon11
- $untaint_ack_msgs <at> show_watch <at> no_watch $show_watch_strict
+ $untaint_all_msgs <at> show_watch <at> no_watch $show_watch_strict
$required_mon_client_version);
# Formatting-related global vars
use vars qw($BGCOLOR $TEXTCOLOR $LINKCOLOR $VLINKCOLOR
<at> <at> -224,7 +224,7 <at> <at>
$cookie_name = "mon-cookie"; #name of cookie given to browser for auth
$cookie_path = "/"; # path for auth cookie
# Set this to "" for auto-path set
- $untaint_ack_msgs = "yes"; # Use HTML::Entities to scrub user-supplied ack messages (recommended!)
+ $untaint_all_msgs = "yes"; # Use HTML::Entities to scrub user-supplied ack messages (recommended!)
# Define optional regexes in the <at> show_watch variable,
# and only hostgroups which match one of these regexes
# will be shown.
<at> <at> -367,10 +367,10 <at> <at>
#
# Used to escape HTML in ack's
#
-if ($untaint_ack_msgs =~ /^y(es)?$/i) {
+if ($untaint_all_msgs =~ /^y(es)?$/i) {
eval "use HTML::Entities" ;
} else {
- undef $untaint_ack_msgs;
+ undef $untaint_all_msgs;
}
<at> <at> -1104,7 +1104,7 <at> <at>
# user requested it, otherwise, just pass it on through
# as is.
if ( $op{$group}{$service}{'ack'} != 0 ) {
- if ($untaint_ack_msgs) {
+ if ($untaint_all_msgs) {
#
# We untaint
#
<at> <at> -1186,7 +1186,8 <at> <at>
$webpage->print("<td align=left bgcolor=\"$td_bg_color\">");
$webpage->print("$service_disabled_string<a href=\"$url?${monhost_and_port_args}command=svc_details&args=$group,$service\">");
$webpage->print("<font size=+1><b>${service}</b></font></a>${desc_string} : \n");
- $webpage->print("<font size=+1>$s->{last_summary}</font>\n");
+ $webpage->print("<font size=+1>" .
+ $untaint_all_msgs ? HTML::Entities::encode_entities($s->{last_summary}) :
$s->{last_summary} . "</font>\n");
$webpage->print("<br>($failure_string)");
$webpage->print(" ${service_acked_string}") if $service_acked_string ne "";
$webpage->print("</td>\n");
<at> <at> -1642,9 +1643,18 <at> <at>
$webpage->print("</td></tr>");
# Now print the detail and summary information for the failed service
- $op{$group}->{$service}->{'last_summary'} = "<not specified>" if
$op{$group}->{$service}->{'last_summary'} eq "" ;
- $op{$group}->{$service}->{'last_detail'} = "<not specified>" if
$op{$group}->{$service}->{'last_detail'} eq "" ;
- $op{$group}->{$service}->{'last_detail'} =~ s/\n/<BR>/g;
+ if ($op{$group}->{$service}->{'last_summary'} eq "") {
+ $op{$group}->{$service}->{'last_summary'} = "<not specified>";
+ } elsif ($untaint_all_msgs) {
+ $op{$group}->{$service}->{'last_summary'} = HTML::Entities::encode_entities($op{$group}{$service}{'last_summary'})
+ }
+ if ($op{$group}->{$service}->{'last_detail'} eq "" ) {
+ $op{$group}->{$service}->{'last_detail'} = "<not specified>"
+ } elsif ($untaint_all_msgs) {
+ $op{$group}->{$service}->{'last_detail'} = HTML::Entities::encode_entities($op{$group}{$service}{'last_detail'})
+ } else { # not much of an untaint
+ $op{$group}->{$service}->{'last_detail'} =~ s/\n/<BR>/g;
+ };
$webpage->print("<tr><td width=25%><font size=+1 color=\"$font_color\">Failure summary</font>:</td><td>$op{$group}->{$service}->{'last_summary'}</td></tr>\n");
$webpage->print("<tr><td width=25%><font size=+1 color=\"$font_color\">Failure detail</font>:</td><td>$op{$group}->{$service}->{'last_detail'}</td></tr>\n");
$webpage->print("</table>");
<at> <at> -3651,8 +3661,8 <at> <at>
return 0;
}
$dtlog_max_failures_per_page = $val;
- } elsif ($key eq "untaint_ack_msgs") {
- $untaint_ack_msgs = $val;
+ } elsif ($key eq "untaint_all_msgs") {
+ $untaint_all_msgs = $val;
} elsif ($key eq "watch") {
push( <at> show_watch, $val);
} elsif ($key eq "show_watch_strict") {
_______________________________________________
mon mailing list
mon <at> linux.kernel.org
http://linux.kernel.org/mailman/listinfo/mon
RSS Feed