Bug 419014: (CVE-2010-3764) [SECURITY] Old charts are not project specific, and product names are viewable in graphs/
r=wurblzap a=LpSolit git-svn-id: svn://10.0.0.236/branches/BUGZILLA-3_6-BRANCH@261505 18797224-902f-48f8-a5cc-f745e15eee43
This commit is contained in:
@@ -1 +1 @@
|
||||
7198
|
||||
7199
|
||||
@@ -543,6 +543,7 @@ sub bz_locations {
|
||||
'datadir' => "$libpath/$datadir",
|
||||
'attachdir' => "$libpath/$datadir/attachments",
|
||||
'skinsdir' => "$libpath/skins",
|
||||
'graphsdir' => "$libpath/graphs",
|
||||
# $webdotdir must be in the web server's tree somewhere. Even if you use a
|
||||
# local dot, we output images to there. Also, if $webdotdir is
|
||||
# not relative to the bugzilla root directory, you'll need to
|
||||
|
||||
@@ -74,6 +74,7 @@ sub FILESYSTEM {
|
||||
my $extlib = bz_locations()->{'ext_libpath'};
|
||||
my $skinsdir = bz_locations()->{'skinsdir'};
|
||||
my $localconfig = bz_locations()->{'localconfig'};
|
||||
my $graphsdir = bz_locations()->{'graphsdir'};
|
||||
|
||||
# We want to set the permissions the same for all localconfig files
|
||||
# across all PROJECTs, so we do something special with $localconfig,
|
||||
@@ -176,7 +177,7 @@ sub FILESYSTEM {
|
||||
dirs => $ws_dir_writeable },
|
||||
$webdotdir => { files => $ws_writeable,
|
||||
dirs => $ws_dir_writeable },
|
||||
graphs => { files => $ws_writeable,
|
||||
$graphsdir => { files => $ws_writeable,
|
||||
dirs => $ws_dir_writeable },
|
||||
|
||||
# Readable directories
|
||||
@@ -228,7 +229,7 @@ sub FILESYSTEM {
|
||||
"$datadir/extensions" => $ws_dir_readable,
|
||||
$attachdir => $ws_dir_writeable,
|
||||
$extensionsdir => $ws_dir_readable,
|
||||
graphs => $ws_dir_writeable,
|
||||
$graphsdir => $ws_dir_writeable,
|
||||
$webdotdir => $ws_dir_writeable,
|
||||
"$skinsdir/custom" => $ws_dir_readable,
|
||||
"$skinsdir/contrib" => $ws_dir_readable,
|
||||
@@ -331,8 +332,17 @@ EOT
|
||||
# in a subdirectory.
|
||||
deny from all
|
||||
EOT
|
||||
},
|
||||
|
||||
"$graphsdir/.htaccess" => { perms => $ws_readable, contents => <<EOT
|
||||
# Allow access to .png and .gif files.
|
||||
<FilesMatch (\\.gif|\\.png)\$>
|
||||
Allow from all
|
||||
</FilesMatch>
|
||||
|
||||
# And no directory listings, either.
|
||||
Deny from all
|
||||
EOT
|
||||
},
|
||||
);
|
||||
|
||||
@@ -358,10 +368,11 @@ sub update_filesystem {
|
||||
my %files = %{$fs->{create_files}};
|
||||
|
||||
my $datadir = bz_locations->{'datadir'};
|
||||
my $graphsdir = bz_locations->{'graphsdir'};
|
||||
# If the graphs/ directory doesn't exist, we're upgrading from
|
||||
# a version old enough that we need to update the $datadir/mining
|
||||
# format.
|
||||
if (-d "$datadir/mining" && !-d 'graphs') {
|
||||
if (-d "$datadir/mining" && !-d $graphsdir) {
|
||||
_update_old_charts($datadir);
|
||||
}
|
||||
|
||||
|
||||
@@ -49,9 +49,12 @@ use Bugzilla::Field;
|
||||
# in the regenerate mode).
|
||||
$| = 1;
|
||||
|
||||
my $datadir = bz_locations()->{'datadir'};
|
||||
my $graphsdir = bz_locations()->{'graphsdir'};
|
||||
|
||||
# Tidy up after graphing module
|
||||
my $cwd = Cwd::getcwd();
|
||||
if (chdir("graphs")) {
|
||||
if (chdir($graphsdir)) {
|
||||
unlink <./*.gif>;
|
||||
unlink <./*.png>;
|
||||
# chdir("..") doesn't work if graphs is a symlink, see bug 429378
|
||||
@@ -68,8 +71,6 @@ if ($#ARGV >= 0 && $ARGV[0] eq "--regenerate") {
|
||||
$regenerate = 1;
|
||||
}
|
||||
|
||||
my $datadir = bz_locations()->{'datadir'};
|
||||
|
||||
my @myproducts = map {$_->name} Bugzilla::Product->get_all;
|
||||
unshift(@myproducts, "-All-");
|
||||
|
||||
|
||||
@@ -45,31 +45,28 @@ use Bugzilla::Util;
|
||||
use Bugzilla::Error;
|
||||
use Bugzilla::Status;
|
||||
|
||||
use File::Basename;
|
||||
use Digest::MD5 qw(md5_hex);
|
||||
|
||||
# If we're using bug groups for products, we should apply those restrictions
|
||||
# to viewing reports, as well. Time to check the login in that case.
|
||||
my $user = Bugzilla->login();
|
||||
my $cgi = Bugzilla->cgi;
|
||||
my $template = Bugzilla->template;
|
||||
my $vars = {};
|
||||
|
||||
if (!Bugzilla->feature('old_charts')) {
|
||||
ThrowCodeError('feature_disabled', { feature => 'old_charts' });
|
||||
}
|
||||
|
||||
my $dir = bz_locations()->{'datadir'} . "/mining";
|
||||
my $graph_url = 'graphs';
|
||||
my $graph_dir = bz_locations()->{'libpath'} . '/' .$graph_url;
|
||||
my $graph_dir = bz_locations()->{'graphsdir'};
|
||||
my $graph_url = basename($graph_dir);
|
||||
my $product_name = $cgi->param('product') || '';
|
||||
|
||||
Bugzilla->switch_to_shadow_db();
|
||||
|
||||
my $cgi = Bugzilla->cgi;
|
||||
my $template = Bugzilla->template;
|
||||
my $vars = {};
|
||||
|
||||
# We only want those products that the user has permissions for.
|
||||
my @myproducts;
|
||||
push( @myproducts, "-All-");
|
||||
# Extract product names from objects and add them to the list.
|
||||
push( @myproducts, map { $_->name } @{$user->get_selectable_products} );
|
||||
|
||||
if (! defined $cgi->param('product')) {
|
||||
if (!$product_name) {
|
||||
# Can we do bug charts?
|
||||
(-d $dir && -d $graph_dir)
|
||||
|| ThrowCodeError('chart_dir_nonexistent',
|
||||
@@ -87,51 +84,62 @@ if (! defined $cgi->param('product')) {
|
||||
push(@datasets, $datasets);
|
||||
}
|
||||
|
||||
# We only want those products that the user has permissions for.
|
||||
my @myproducts = ('-All-');
|
||||
# Extract product names from objects and add them to the list.
|
||||
push( @myproducts, map { $_->name } @{$user->get_selectable_products} );
|
||||
|
||||
$vars->{'datasets'} = \@datasets;
|
||||
$vars->{'products'} = \@myproducts;
|
||||
|
||||
print $cgi->header();
|
||||
|
||||
$template->process('reports/old-charts.html.tmpl', $vars)
|
||||
|| ThrowTemplateError($template->error());
|
||||
exit;
|
||||
}
|
||||
else {
|
||||
my $product = $cgi->param('product');
|
||||
|
||||
# For security and correctness, validate the value of the "product" form variable.
|
||||
# Valid values are those products for which the user has permissions which appear
|
||||
# in the "product" drop-down menu on the report generation form.
|
||||
grep($_ eq $product, @myproducts)
|
||||
|| ThrowUserError("invalid_product_name", {product => $product});
|
||||
my ($product) = grep { $_->name eq $product_name } @{$user->get_selectable_products};
|
||||
($product || $product_name eq '-All-')
|
||||
|| ThrowUserError('invalid_product_name', {product => $product_name});
|
||||
|
||||
# We've checked that the product exists, and that the user can see it
|
||||
# This means that is OK to detaint
|
||||
trick_taint($product);
|
||||
# Product names can change over time. Their ID cannot; so use the ID
|
||||
# to generate the filename.
|
||||
my $prod_id = $product ? $product->id : 0;
|
||||
|
||||
defined($cgi->param('datasets')) || ThrowUserError('missing_datasets');
|
||||
# Make sure there is something to plot.
|
||||
my @datasets = $cgi->param('datasets');
|
||||
scalar(@datasets) || ThrowUserError('missing_datasets');
|
||||
|
||||
my $datasets = join('', $cgi->param('datasets'));
|
||||
|
||||
my $type = chart_image_type();
|
||||
my $data_file = daily_stats_filename($product);
|
||||
my $image_file = chart_image_name($data_file, $type, $datasets);
|
||||
my $url_image = correct_urlbase() . "$graph_url/$image_file";
|
||||
|
||||
if (! -e "$graph_dir/$image_file") {
|
||||
generate_chart("$dir/$data_file", "$graph_dir/$image_file", $type,
|
||||
$product, $datasets);
|
||||
if (grep { $_ !~ /^[A-Za-z0-9:_-]+$/ } @datasets) {
|
||||
ThrowUserError('invalid_datasets', {'datasets' => \@datasets});
|
||||
}
|
||||
|
||||
$vars->{'url_image'} = $url_image;
|
||||
# Filenames must not be guessable as they can point to products
|
||||
# you are not allowed to see. Also, different projects can have
|
||||
# the same product names.
|
||||
my $key = Bugzilla->localconfig->{'site_wide_secret'};
|
||||
my $project = bz_locations()->{'project'} || '';
|
||||
my $image_file = join(':', ($key, $project, $prod_id, @datasets));
|
||||
# Wide characters cause md5_hex() to die.
|
||||
if (Bugzilla->params->{'utf8'}) {
|
||||
utf8::encode($image_file) if utf8::is_utf8($image_file);
|
||||
}
|
||||
my $type = chart_image_type();
|
||||
$image_file = md5_hex($image_file) . ".$type";
|
||||
trick_taint($image_file);
|
||||
|
||||
if (! -e "$graph_dir/$image_file") {
|
||||
generate_chart($dir, "$graph_dir/$image_file", $type, $product, \@datasets);
|
||||
}
|
||||
|
||||
$vars->{'url_image'} = "$graph_url/$image_file";
|
||||
|
||||
print $cgi->header(-Content_Disposition=>'inline; filename=bugzilla_report.html');
|
||||
|
||||
$template->process('reports/old-charts.html.tmpl', $vars)
|
||||
|| ThrowTemplateError($template->error());
|
||||
exit;
|
||||
}
|
||||
|
||||
$template->process('reports/old-charts.html.tmpl', $vars)
|
||||
|| ThrowTemplateError($template->error());
|
||||
|
||||
#####################
|
||||
# Subroutines #
|
||||
#####################
|
||||
@@ -140,9 +148,8 @@ sub get_data {
|
||||
my $dir = shift;
|
||||
|
||||
my @datasets;
|
||||
my $datafile = daily_stats_filename('-All-');
|
||||
open(DATA, '<', "$dir/$datafile")
|
||||
|| ThrowCodeError('chart_file_open_fail', {filename => "$dir/$datafile"});
|
||||
open(DATA, '<', "$dir/-All-")
|
||||
|| ThrowCodeError('chart_file_open_fail', {filename => "$dir/-All-"});
|
||||
|
||||
while (<DATA>) {
|
||||
if (/^# fields?: (.+)\s*$/) {
|
||||
@@ -154,12 +161,6 @@ sub get_data {
|
||||
return @datasets;
|
||||
}
|
||||
|
||||
sub daily_stats_filename {
|
||||
my ($prodname) = @_;
|
||||
$prodname =~ s/\//-/gs;
|
||||
return $prodname;
|
||||
}
|
||||
|
||||
sub chart_image_type {
|
||||
# what chart type should we be generating?
|
||||
my $testimg = Chart::Lines->new(2,2);
|
||||
@@ -169,32 +170,12 @@ sub chart_image_type {
|
||||
return $type;
|
||||
}
|
||||
|
||||
sub chart_image_name {
|
||||
my ($data_file, $type, $datasets) = @_;
|
||||
|
||||
# This routine generates a filename from the requested fields. The problem
|
||||
# is that we have to check the safety of doing this. We can't just require
|
||||
# that the fields exist, because what stats were collected could change
|
||||
# over time (eg by changing the resolutions available)
|
||||
# Instead, just require that each field name consists only of letters,
|
||||
# numbers, underscores and hyphens.
|
||||
|
||||
if ($datasets !~ m/^[A-Za-z0-9:_-]+$/) {
|
||||
ThrowUserError('invalid_datasets', {'datasets' => $datasets});
|
||||
}
|
||||
|
||||
# Since we pass the tests, consider it OK
|
||||
trick_taint($datasets);
|
||||
|
||||
# Cache charts by generating a unique filename based on what they
|
||||
# show. Charts should be deleted by collectstats.pl nightly.
|
||||
my $id = join ("_", split (":", $datasets));
|
||||
|
||||
return "${data_file}_${id}.$type";
|
||||
}
|
||||
|
||||
sub generate_chart {
|
||||
my ($data_file, $image_file, $type, $product, $datasets) = @_;
|
||||
my ($dir, $image_file, $type, $product, $datasets) = @_;
|
||||
$product = $product ? $product->name : '-All-';
|
||||
my $data_file = $product;
|
||||
$data_file =~ s/\//-/gs;
|
||||
$data_file = $dir . '/' . $data_file;
|
||||
|
||||
if (! open FILE, $data_file) {
|
||||
if ($product eq '-All-') {
|
||||
@@ -205,7 +186,7 @@ sub generate_chart {
|
||||
|
||||
my @fields;
|
||||
my @labels = qw(DATE);
|
||||
my %datasets = map { $_ => 1 } split /:/, $datasets;
|
||||
my %datasets = map { $_ => 1 } @$datasets;
|
||||
|
||||
my %data = ();
|
||||
while (<FILE>) {
|
||||
|
||||
@@ -919,7 +919,7 @@
|
||||
|
||||
[% ELSIF error == "invalid_datasets" %]
|
||||
[% title = "Invalid Datasets" %]
|
||||
Invalid datasets <em>[% datasets FILTER html %]</em>. Only digits,
|
||||
Invalid datasets <em>[% datasets.join(":") FILTER html %]</em>. Only digits,
|
||||
letters and colons are allowed.
|
||||
|
||||
[% ELSIF error == "invalid_format" %]
|
||||
|
||||
@@ -51,7 +51,7 @@
|
||||
[%# We cannot use translated statuses and resolutions from field-descs.none.html
|
||||
# because old charts do not distinguish statuses from resolutions. %]
|
||||
[% FOREACH dataset = datasets %]
|
||||
<option value="[% dataset.value FILTER html %]:"
|
||||
<option value="[% dataset.value FILTER html %]"
|
||||
[% " selected=\"selected\"" IF dataset.selected %]>
|
||||
[% dataset.value FILTER html %]</option>
|
||||
[% END %]
|
||||
|
||||
Reference in New Issue
Block a user