Skip to content

bugzilla
bugzilla
Commit c793bb32 authored by Frédéric Buclin's avatar Frédéric Buclin
Browse files
Options

Bug 419014: (CVE-2010-3764) [SECURITY] Old charts are not project specific, and…

Bug 419014: (CVE-2010-3764) [SECURITY] Old charts are not project specific, and product names are viewable in graphs/ r=wurblzap a=LpSolit
parent 480a9b8e
Hide whitespace changes
Inline Side-by-side
Showing with 75 additions and 78 deletions
Bugzilla/Constants.pm
View file @ c793bb32
......@@ -623,6 +623,7 @@ sub bz_locations {
'datadir' => $datadir,
'attachdir' => "$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
......
Bugzilla/Install/Filesystem.pm
View file @ c793bb32
......@@ -122,6 +122,7 @@ sub FILESYSTEM {
my $skinsdir = bz_locations()->{'skinsdir'};
my $localconfig = bz_locations()->{'localconfig'};
my $template_cache = bz_locations()->{'template_cache'};
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,
......@@ -197,7 +198,7 @@ sub FILESYSTEM {
dirs => DIR_CGI_WRITE },
$webdotdir => { files => WS_SERVE,
dirs => DIR_CGI_WRITE | DIR_ALSO_WS_SERVE },
graphs => { files => WS_SERVE,
$graphsdir => { files => WS_SERVE,
dirs => DIR_CGI_WRITE | DIR_ALSO_WS_SERVE },
"$datadir/db" => { files => CGI_WRITE,
dirs => DIR_CGI_WRITE },
......@@ -269,7 +270,7 @@ sub FILESYSTEM {
# Directories that cgi scripts can write to.
"$datadir/db" => DIR_CGI_WRITE,
$attachdir => DIR_CGI_WRITE,
graphs => DIR_CGI_WRITE | DIR_ALSO_WS_SERVE,
$graphsdir => DIR_CGI_WRITE | DIR_ALSO_WS_SERVE,
$webdotdir => DIR_CGI_WRITE | DIR_ALSO_WS_SERVE,
# Directories that contain content served directly by the web server.
"$skinsdir/custom" => DIR_WS_SERVE,
......@@ -331,6 +332,17 @@ EOT
"$datadir/.htaccess" => { perms => WS_SERVE,
contents => HT_DEFAULT_DENY },
"$graphsdir/.htaccess" => { perms => WS_SERVE, 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
},
"$webdotdir/.htaccess" => { perms => WS_SERVE, contents => <<EOT
# Restrict access to .dot files to the public webdot server at research.att.com
# if research.att.com ever changes their IP, or if you use a different
......@@ -373,10 +385,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);
}
......
collectstats.pl
View file @ c793bb32
......@@ -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'};
# As we can now customize statuses and resolutions, looking at the current list
# of legal values only is not enough as some now removed statuses and resolutions
# may have existed in the past, or have been renamed. We want them all.
......
reports.cgi
View file @ c793bb32
......@@ -38,31 +38,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',
......@@ -80,49 +77,61 @@ 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'));
if (grep { $_ !~ /^[A-Za-z0-9:_-]+$/ } @datasets) {
ThrowUserError('invalid_datasets', {'datasets' => \@datasets});
}
my $data_file = daily_stats_filename($product);
my $image_file = chart_image_name($data_file, $datasets);
my $url_image = correct_urlbase() . "$graph_url/$image_file";
# 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);
}
$image_file = md5_hex($image_file) . '.png';
trick_taint($image_file);
if (! -e "$graph_dir/$image_file") {
generate_chart("$dir/$data_file", "$graph_dir/$image_file", $product, $datasets);
generate_chart($dir, "$graph_dir/$image_file", $product, \@datasets);
}
$vars->{'url_image'} = $url_image;
$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 #
#####################
......@@ -131,9 +140,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*$/) {
......@@ -145,38 +153,12 @@ sub get_data {
return @datasets;
}
sub daily_stats_filename {
my ($prodname) = @_;
$prodname =~ s/\//-/gs;
return $prodname;
}
sub chart_image_name {
my ($data_file, $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}.png";
}
sub generate_chart {
my ($data_file, $image_file, $product, $datasets) = @_;
my ($dir, $image_file, $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-') {
......@@ -187,7 +169,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>) {
......
template/en/default/global/user-error.html.tmpl
View file @ c793bb32
......@@ -907,7 +907,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" %]
......
template/en/default/reports/old-charts.html.tmpl
View file @ c793bb32
......@@ -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 %]
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment