Another difference is that upstream 2.6.9 used a replacement character
of underscore rather than a dot. Attached is my suggested revision of
Salvatore's patch (also adds filtering of time specifiers).
I've tested this on an existing wheezy/sid SmokePing installation; it
stops the injection of quotes into the <img> tag I demonstrated before.
It also prevents those characters from being used in graph filenames in
the cache directory. I've tried some valid time specifiers and they are
still working.
Regards,
--
Steven Chamberlain
ste...@pyro.eu.org
Index: smokeping-2.6.8/lib/Smokeping.pm
===================================================================
--- smokeping-2.6.8.orig/lib/Smokeping.pm 2012-02-26 18:19:45.000000000 +0000
+++ smokeping-2.6.8/lib/Smokeping.pm 2013-03-16 23:07:00.000000000 +0000
@@ -28,6 +28,8 @@
# make sure we do not end up with , in odd places where one would expect a '.'
# we set the environment variable so that our 'kids' get the benefit too
+my $xssBadRx = qr/[<>%&'";]/;
+
$ENV{'LC_NUMERIC'}='C';
if (setlocale(LC_NUMERIC,"") ne "C") {
if ($ENV{'LC_ALL'} eq 'C') {
@@ -170,7 +172,7 @@
my $hierarchy = '';
my $h = $q->param('hierarchy');
if ($q->param('hierarchy')){
- $h =~ s/[<>&%]/./g;
+ $h =~ s/$xssBadRx/_/g;
$hierarchy = 'hierarchy='.$h.';';
};
return $hierarchy;
@@ -212,7 +214,7 @@
my $address = $ENV{REMOTE_ADDR};
my $targetptr = $cfg->{Targets};
foreach my $step (@target){
- $step =~ s/[<>&%]/./g;
+ $step =~ s/$xssBadRx/_/g;
return "Error: Unknown target $step"
unless defined $targetptr->{$step};
$targetptr = $targetptr->{$step};
@@ -1024,6 +1026,7 @@
sub parse_datetime($){
my $in = shift;
for ($in){
+ $in =~ s/$xssBadRx/_/g;
/^(\d+)$/ && do { my $value = $1; $value = time if $value > 2**32; return $value};
/^\s*(\d{4})-(\d{1,2})-(\d{1,2})(?:\s+(\d{1,2}):(\d{2})(?::(\d{2}))?)?\s*$/ &&
return POSIX::mktime($6||0,$5||0,$4||0,$3,$2-1,$1-1900,0,0,-1);
@@ -1047,7 +1050,7 @@
my $tree = shift;
my $open = shift;
my $mode = shift || $q->param('displaymode') || 's';
- $mode =~ s/[<>&%]/./g;
+ $mode =~ s/$xssBadRx/_/g;
my $phys_tree = $tree;
my $phys_open = $open;
if ($tree->{__tree_link}){
@@ -1447,7 +1450,7 @@
$startstr =~ s/\s/%20/g;
$endstr =~ s/\s/%20/g;
my $t = $q->param('target');
- $t =~ s/[<>&%]/./g;
+ $t =~ s/$xssBadRx/_/g;
for my $slave (@slaves){
my $s = $slave ? "~$slave" : "";
$page .= "<div>";
@@ -1601,7 +1604,7 @@
my $t = $q->param('target');
if ( $t and $t !~ /\.\./ and $t =~ /(\S+)/){
$targ = $1;
- $targ =~ s/[<>;%]/./g;
+ $targ =~ s/$xssBadRx/_/g;
}
my ($path,$slave) = split(/~/,$targ);
if ($slave and $slave =~ /(\S+)/){
@@ -1610,7 +1613,7 @@
$slave = $1;
}
my $hierarchy = $q->param('hierarchy');
- $hierarchy =~ s/[<>;%]/./g;
+ $hierarchy =~ s/$xssBadRx/_/g;
die "ERROR: unknown hierarchy $hierarchy\n"
if $hierarchy and not $cfg->{Presentation}{hierarchies}{$hierarchy};
my $open = [ (split /\./,$path||'') ];
