Skip to content

bugzilla
bugzilla
Commit 9042b58f authored by bbaetz%cs.mcgill.ca's avatar bbaetz%cs.mcgill.ca
Browse files
Options

Bug 97469 - Assignee/QA/Reporter/CC don't get email on restricted bugs.

Also fixes seeing bugs in the buglist (bug 95024), dependancy lists, tooltips, duplicates, and everywhere else I could see which checked group bugs.groupset == 0. Also fxed bug 101560, by clearing BASH_ENV r=myk,justdave
parent 35f74cea
Hide whitespace changes
Inline Side-by-side
Showing with 177 additions and 158 deletions
Bug.pm
View file @ 9042b58f
......@@ -113,10 +113,9 @@ sub initBug {
groupset, delta_ts, sum(votes.count)
from bugs left join votes using(bug_id)
where bugs.bug_id = $bug_id
and bugs.groupset & $usergroupset = bugs.groupset
group by bugs.bug_id";
&::SendSQL($query);
&::SendSQL(&::SelectVisible($query, $user_id, $usergroupset));
my @row;
if (@row = &::FetchSQLData()) {
......@@ -445,6 +444,7 @@ sub Collision {
my $write = "WRITE"; # Might want to make a param to control
# whether we do LOW_PRIORITY ...
&::SendSQL("LOCK TABLES bugs $write, bugs_activity $write, cc $write, " .
"cc AS selectVisible_cc $write, " .
"profiles $write, dependencies $write, votes $write, " .
"keywords $write, longdescs $write, fielddefs $write, " .
"keyworddefs READ, groups READ, attachments READ, products READ");
......
Bugzilla/Bug.pm
View file @ 9042b58f
......@@ -113,10 +113,9 @@ sub initBug {
groupset, delta_ts, sum(votes.count)
from bugs left join votes using(bug_id)
where bugs.bug_id = $bug_id
and bugs.groupset & $usergroupset = bugs.groupset
group by bugs.bug_id";
&::SendSQL($query);
&::SendSQL(&::SelectVisible($query, $user_id, $usergroupset));
my @row;
if (@row = &::FetchSQLData()) {
......@@ -445,6 +444,7 @@ sub Collision {
my $write = "WRITE"; # Might want to make a param to control
# whether we do LOW_PRIORITY ...
&::SendSQL("LOCK TABLES bugs $write, bugs_activity $write, cc $write, " .
"cc AS selectVisible_cc $write, " .
"profiles $write, dependencies $write, votes $write, " .
"keywords $write, longdescs $write, fielddefs $write, " .
"keyworddefs READ, groups READ, attachments READ, products READ");
......
CGI.pl
View file @ 9042b58f
......@@ -258,88 +258,20 @@ sub ValidateBugID {
# setting those local variables to the default value of zero if
# the global variables are undefined.
# "usergroupset" stores the set of groups the user is a member of,
# while "userid" stores the user's unique ID. These variables are
# set globally by either confirm_login() or quietly_check_login(),
# one of which should be run before calling this function; otherwise
# this function will treat the user as if they were not logged in
# and throw an error if they try to access a bug that requires
# permissions/authorization to access.
my $usergroupset = $::usergroupset || 0;
my $userid = $::userid || 0;
# Query the database for the bug, retrieving a boolean value that
# represents whether or not the user is authorized to access the bug.
# Users are authorized to access bugs if they are a member of all
# groups to which the bug is restricted. User group membership and
# bug restrictions are stored as bits within bitsets, so authorization
# can be determined by comparing the intersection of the user's
# bitset with the bug's bitset. If the result matches the bug's bitset
# the user is a member of all groups to which the bug is restricted
# and is authorized to access the bug.
# A user is also authorized to access a bug if she is the reporter,
# assignee, QA contact, or member of the cc: list of the bug and the bug
# allows users in those roles to see the bug. The boolean fields
# reporter_accessible, assignee_accessible, qacontact_accessible, and
# cclist_accessible identify whether or not those roles can see the bug.
# Bit arithmetic is performed by MySQL instead of Perl because bitset
# fields in the database are 64 bits wide (BIGINT), and Perl installations
# may or may not support integers larger than 32 bits. Using bitsets
# and doing bitset arithmetic is probably not cross-database compatible,
# however, so these mechanisms are likely to change in the future.
# Get data from the database about whether or not the user belongs to
# all groups the bug is in, and who are the bug's reporter and qa_contact
# along with which roles can always access the bug.
SendSQL("SELECT ((groupset & $usergroupset) = groupset) , reporter , assigned_to , qa_contact ,
reporter_accessible , assignee_accessible , qacontact_accessible , cclist_accessible
FROM bugs
WHERE bug_id = $id");
# Make sure the bug exists in the database.
MoreSQLData()
|| DisplayError("Bug #$id does not exist.")
&& exit;
my ($isauthorized, $reporter, $assignee, $qacontact, $reporter_accessible,
$assignee_accessible, $qacontact_accessible, $cclist_accessible) = FetchSQLData();
# First check that the bug exists
SendSQL("SELECT bug_id FROM bugs WHERE bug_id = $id");
# Finish validation and return if the user is a member of all groups to which the bug belongs.
return if $isauthorized;
# Finish validation and return if the user is in a role that has access to the bug.
if ($userid) {
return
if ($reporter_accessible && $reporter == $userid)
|| ($assignee_accessible && $assignee == $userid)
|| ($qacontact_accessible && $qacontact == $userid);
}
# Try to authorize the user one more time by seeing if they are on
# the cc: list. If so, finish validation and return.
if ( $cclist_accessible ) {
my @cclist;
SendSQL("SELECT cc.who
FROM bugs , cc
WHERE bugs.bug_id = $id
AND cc.bug_id = bugs.bug_id
");
while (my ($ccwho) = FetchSQLData()) {
# more efficient to just check the var here instead of
# creating a potentially huge array to grep against
return if ($userid == $ccwho);
}
FetchOneColumn()
|| DisplayError("Bug #$id does not exist.")
&& exit;
}
return if CanSeeBug($id, $::userid, $::usergroupset);
# The user did not pass any of the authorization tests, which means they
# are not authorized to see the bug. Display an error and stop execution.
# The error the user sees depends on whether or not they are logged in
# (i.e. $userid contains the user's positive integer ID).
if ($userid) {
# (i.e. $::userid contains the user's positive integer ID).
if ($::userid) {
DisplayError("You are not authorized to access bug #$id.");
} else {
DisplayError(
......
buglist.cgi
View file @ 9042b58f
......@@ -37,6 +37,7 @@ sub sillyness {
$zz = $::db_name;
$zz = $::defaultqueryname;
$zz = $::unconfirmedstate;
$zz = $::userid;
$zz = @::components;
$zz = @::default_column_list;
$zz = @::legal_keywords;
......@@ -162,9 +163,7 @@ sub GenerateSQL {
"LEFT JOIN profiles map_qa_contact ON bugs.qa_contact = map_qa_contact.userid"));
unshift(@wherepart,
("bugs.assigned_to = map_assigned_to.userid",
"bugs.reporter = map_reporter.userid",
"bugs.groupset & $::usergroupset = bugs.groupset"));
"bugs.reporter = map_reporter.userid"));
my $minvotes;
if (defined $F{'votes'}) {
......@@ -805,10 +804,14 @@ sub GenerateSQL {
$suppseen{$str} = 1;
}
}
my $query = ("SELECT " . join(', ', @fields) .
" FROM $suppstring" .
" WHERE " . join(' AND ', (@wherepart, @andlist)) .
" GROUP BY bugs.bug_id");
$query = SelectVisible($query, $::userid, $::usergroupset);
if ($debug) {
print "<P><CODE>" . value_quote($query) . "</CODE><P>\n";
exit();
......@@ -1069,8 +1072,6 @@ ReconnectToShadowDatabase();
my $query = GenerateSQL(\@fields, undef, undef, $::buffer);
if ($::COOKIE{'LASTORDER'}) {
if ((!$::FORM{'order'}) || $::FORM{'order'} =~ /^reuse/i) {
$::FORM{'order'} = url_decode($::COOKIE{'LASTORDER'});
......@@ -1132,6 +1133,7 @@ if ($::FORM{'debug'} && $serverpush) {
if (Param('expectbigqueries')) {
SendSQL("set option SQL_BIG_TABLES=1");
}
SendSQL($query);
my $count = 0;
......
duplicates.cgi
View file @ 9042b58f
......@@ -33,6 +33,12 @@ require "CGI.pl";
ConnectToDatabase(1);
GetVersionTable();
quietly_check_login();
# Silly used-once warnings
$::userid = $::userid;
$::usergroupset = $::usergroupset;
my %dbmcount;
my %count;
my $dobefore = 0;
......@@ -195,10 +201,10 @@ $i = 0;
foreach (@sortedcount)
{
my $id = $_;
SendSQL("SELECT component, bug_severity, op_sys, target_milestone, short_desc, groupset, bug_status, resolution" .
" FROM bugs WHERE bug_id = $id");
my ($component, $severity, $op_sys, $milestone, $summary, $groupset, $bug_status, $resolution) = FetchSQLData();
next unless $groupset == 0;
SendSQL(SelectVisible("SELECT component, bug_severity, op_sys, target_milestone, short_desc, bug_status, resolution" .
" FROM bugs WHERE bugs.bug_id = $id", $::userid, $::usergroupset));
next unless MoreSQLData();
my ($component, $severity, $op_sys, $milestone, $summary, $bug_status, $resolution) = FetchSQLData();
$summary = html_quote($summary);
# Show all bugs except those CLOSED _OR_ VERIFIED but not INVALID or WONTFIX.
......
globals.pl
View file @ 9042b58f
......@@ -20,6 +20,7 @@
# Contributor(s): Terry Weissman <terry@mozilla.org>
# Dan Mosedale <dmose@mozilla.org>
# Jake <jake@acutex.net>
# Bradley Baetz <bbaetz@cs.mcgill.ca>
# Contains some global variables and routines used throughout bugzilla.
......@@ -71,8 +72,9 @@ use Date::Parse; # For str2time().
#use Carp; # for confess
use RelationSet;
# $ENV{PATH} is not taint safe
# Some environment variables are not taint safe
delete $ENV{PATH};
delete $ENV{BASH_ENV};
# Contains the version string for the current running Bugzilla.
$::param{'version'} = '2.15';
......@@ -704,6 +706,98 @@ sub GenerateRandomPassword {
return $password;
}
sub SelectVisible {
my ($query, $userid, $usergroupset) = @_;
# Run the SQL $query with the additional restriction that
# the bugs can be seen by $userid. $usergroupset is provided
# as an optimisation when this is already known, eg from CGI.pl
# If not present, it will be obtained from the db.
# Assumes that 'bugs' is mentioned as a table name. You should
# also make sure that bug_id is qualified bugs.bug_id!
# Your query must have a WHERE clause. This is unlikely to be a problem.
# Also, note that mySQL requires aliases for tables to be locked, as well
# This means that if you change the name from selectVisible_cc (or add
# additional tables), you will need to update anywhere which does a
# LOCK TABLE, and then calls routines which call this
$usergroupset = 0 unless $userid;
unless (defined($usergroupset)) {
PushGlobalSQLState();
SendSQL("SELECT groupset FROM profiles WHERE userid = $userid");
$usergroupset = FetchOneColumn();
PopGlobalSQLState();
}
# Users are authorized to access bugs if they are a member of all
# groups to which the bug is restricted. User group membership and
# bug restrictions are stored as bits within bitsets, so authorization
# can be determined by comparing the intersection of the user's
# bitset with the bug's bitset. If the result matches the bug's bitset
# the user is a member of all groups to which the bug is restricted
# and is authorized to access the bug.
# A user is also authorized to access a bug if she is the reporter,
# assignee, QA contact, or member of the cc: list of the bug and the bug
# allows users in those roles to see the bug. The boolean fields
# reporter_accessible, assignee_accessible, qacontact_accessible, and
# cclist_accessible identify whether or not those roles can see the bug.
# Bit arithmetic is performed by MySQL instead of Perl because bitset
# fields in the database are 64 bits wide (BIGINT), and Perl installations
# may or may not support integers larger than 32 bits. Using bitsets
# and doing bitset arithmetic is probably not cross-database compatible,
# however, so these mechanisms are likely to change in the future.
my $replace = " ";
if ($userid) {
$replace .= "LEFT JOIN cc selectVisible_cc ON
bugs.bug_id = selectVisible_cc.bug_id AND
selectVisible_cc.who = $userid "
}
$replace .= "WHERE ((bugs.groupset & $usergroupset) = bugs.groupset ";
if ($userid) {
# There is a mysql bug affecting v3.22 and 3.23 (at least), where this will
# cause all rows to be returned! We work arround this by adding an not isnull
# test to the JOINed cc table. See http://lists.mysql.com/cgi-ez/ezmlm-cgi?9:mss:11417
# Its needed, even though it shouldn't be
$replace .= "OR (bugs.reporter_accessible = 1 AND bugs.reporter = $userid)
OR (bugs.assignee_accessible = 1 AND bugs.assigned_to = $userid)
OR (bugs.qacontact_accessible = 1 AND bugs.qa_contact = $userid)
OR (bugs.cclist_accessible = 1 AND selectVisible_cc.who = $userid AND not isnull(selectVisible_cc.who))";
}
$replace .= ") AND ";
$query =~ s/\sWHERE\s/$replace/i;
return $query;
}
sub CanSeeBug {
# Note that we pass in the usergroupset, since this is known
# in most cases (ie viewing bugs). Maybe make this an optional
# parameter?
my ($id, $userid, $usergroupset) = @_;
# Query the database for the bug, retrieving a boolean value that
# represents whether or not the user is authorized to access the bug.
PushGlobalSQLState();
SendSQL(SelectVisible("SELECT bugs.bug_id FROM bugs WHERE bugs.bug_id = $id",
$userid, $usergroupset));
my $ret = defined(FetchSQLData());
PopGlobalSQLState();
return $ret;
}
sub ValidatePassword {
# Determines whether or not a password is valid (i.e. meets Bugzilla's
......@@ -835,16 +929,22 @@ sub DBNameToIdAndCheck {
exit(0);
}
# Use detaint_string() when you know that there is no way that the data
# Use trick_taint() when you know that there is no way that the data
# in a scalar can be tainted, but taint mode still bails on it.
# WARNING!! Using this routine on data that really could be tainted
# defeats the purpose of taint mode. It should only be
# used on variables that cannot be touched by users.
sub detaint_string {
my ($str) = @_;
$str =~ m/^(.*)$/s;
$str = $1;
sub trick_taint {
$_[0] =~ /^(.*)$/s;
$_[0] = $1;
return (defined($_[0]));
}
sub detaint_natural {
$_[0] =~ /^(\d+)$/;
$_[0] = $1;
return (defined($_[0]));
}
# This routine quoteUrls contains inspirations from the HTML::FromText CPAN
......@@ -983,7 +1083,9 @@ sub GetBugLink {
$link_text = value_quote($link_text);
$link_return .= qq{<a href="show_bug.cgi?id=$bug_num" title="$bug_stat};
if ($bug_res ne "") {$link_return .= " $bug_res"}
if ($bug_grp == 0) { $link_return .= " - $bug_desc" }
if ($bug_grp == 0 || CanSeeBug($bug_num, $::userid, $::usergroupset)) {
$link_return .= " - $bug_desc";
}
$link_return .= qq{">$link_text</a>};
if ($bug_res ne "") { $link_return .= "</strike>" }
if ($bug_stat eq "UNCONFIRMED") { $link_return .= "</i>"}
......@@ -1128,7 +1230,7 @@ sub SqlQuote {
$str =~ s/([\\\'])/\\$1/g;
$str =~ s/\0/\\0/g;
# If it's been SqlQuote()ed, then it's safe, so we tell -T that.
$str = detaint_string($str);
trick_taint($str);
return "'$str'";
}
......
long_list.cgi
View file @ 9042b58f
......@@ -32,6 +32,7 @@ require "CGI.pl";
sub sillyness {
my $zz;
$zz = $::legal_keywords;
$zz = $::userid;
$zz = $::usergroupset;
$zz = %::FORM;
}
......@@ -68,12 +69,12 @@ select
bugs.status_whiteboard,
bugs.keywords
from bugs,profiles assign,profiles report
where assign.userid = bugs.assigned_to and report.userid = bugs.reporter and
bugs.groupset & $::usergroupset = bugs.groupset and";
where assign.userid = bugs.assigned_to and report.userid = bugs.reporter and";
$::FORM{'buglist'} = "" unless exists $::FORM{'buglist'};
foreach my $bug (split(/:/, $::FORM{'buglist'})) {
SendSQL("$generic_query bugs.bug_id = $bug");
SendSQL(SelectVisible("$generic_query bugs.bug_id = $bug",
$::userid, $::usergroupset));
my @row;
if (@row = FetchSQLData()) {
......
process_bug.cgi
View file @ 9042b58f
......@@ -352,6 +352,11 @@ empowered user, may make that change to the $f field.
# Confirm that the reporter of the current bug can access the bug we are duping to.
sub DuplicateUserConfirm {
# if we've already been through here, then exit
if (defined $::FORM{'confirm_add_duplicate'}) {
return;
}
my $dupe = trim($::FORM{'id'});
my $original = trim($::FORM{'dup_id'});
......@@ -360,45 +365,13 @@ sub DuplicateUserConfirm {
SendSQL("SELECT profiles.groupset FROM profiles WHERE profiles.userid =".SqlQuote($reporter));
my $reportergroupset = FetchOneColumn();
SendSQL("SELECT ((groupset & $reportergroupset) = groupset) , reporter , assigned_to , qa_contact ,
reporter_accessible , assignee_accessible , qacontact_accessible , cclist_accessible
FROM bugs
WHERE bug_id = $original");
my ($isauthorized, $originalreporter, $assignee, $qacontact, $reporter_accessible,
$assignee_accessible, $qacontact_accessible, $cclist_accessible) = FetchSQLData();
# If reporter is authorized via the database, or is the original reporter, assignee,
# or QA Contact, we'll automatically confirm they can be added to the cc list
if ($isauthorized
|| ($reporter_accessible && $originalreporter == $reporter)
|| ($assignee_accessible && $assignee == $reporter)
|| ($qacontact_accessible && $qacontact == $reporter)) {
if (CanSeeBug($original, $reporter, $reportergroupset)) {
$::FORM{'confirm_add_duplicate'} = "1";
return;
}
# Try to authorize the user one more time by seeing if they are on
# the cc: list. If so, finish validation and return.
if ($cclist_accessible ) {
my @cclist;
SendSQL("SELECT cc.who
FROM bugs , cc
WHERE bugs.bug_id = $original
AND cc.bug_id = bugs.bug_id
");
while (my ($ccwho) = FetchSQLData()) {
if ($reporter == $ccwho) {
$::FORM{'confirm_add_duplicate'} = "1";
return;
}
}
}
if (defined $::FORM{'confirm_add_duplicate'}) {
return;
}
SendSQL("SELECT cclist_accessible FROM bugs WHERE bug_id = $original");
my $cclist_accessible = FetchOneColumn();
# Once in this part of the subroutine, the user has not been auto-validated
# and the duper has not chosen whether or not to add to CC list, so let's
......@@ -985,6 +958,7 @@ foreach my $id (@idlist) {
my $write = "WRITE"; # Might want to make a param to control
# whether we do LOW_PRIORITY ...
SendSQL("LOCK TABLES bugs $write, bugs_activity $write, cc $write, " .
"cc AS selectVisible_cc $write, " .
"profiles $write, dependencies $write, votes $write, " .
"keywords $write, longdescs $write, fielddefs $write, " .
"keyworddefs READ, groups READ, attachments READ, products READ");
......
processmail
View file @ 9042b58f
......@@ -111,8 +111,8 @@ sub ProcessOneBug {
}
my ($start, $end) = (@row);
# $start and $end are considered safe because users can't touch them
$start = detaint_string($start);
$end = detaint_string($end);
trick_taint($start);
trick_taint($end);
my $ccSet = new RelationSet();
$ccSet->mergeFromDB("SELECT who FROM cc WHERE bug_id = $id");
......@@ -644,31 +644,26 @@ sub NewProcessOnePerson ($$$$$$$$$$$) {
if ($nomail{$person}) {
return;
}
# Sanitize $values{'groupset'}
if ($values{'groupset'} =~ m/(\d+)/) {
$values{'groupset'} = $1;
} else {
$values{'groupset'} = 0;
}
SendSQL("SELECT userid, groupset & $values{'groupset'} " .
SendSQL("SELECT userid, groupset " .
"FROM profiles WHERE login_name = " . SqlQuote($person));
my ($userid, $groupset) = (FetchSQLData());
$seen{$person} = 1;
detaint_natural($userid);
detaint_natural($groupset);
# if this person doesn't have permission to see info on this bug,
# return.
#
# XXX - I _think_ this currently means that if a bug is suddenly given
# XXX - This currently means that if a bug is suddenly given
# more restrictive permissions, people without those permissions won't
# see the action of restricting the bug itself; the bug will just
# quietly disappear from their radar.
#
if ($groupset ne $values{'groupset'}) {
return;
}
return unless CanSeeBug($id, $userid, $groupset);
my %mailhead = %defmailhead;
......@@ -824,9 +819,10 @@ if ($ARGV[0] eq "rescanall") {
push @list, $row[0];
}
foreach my $id (@list) {
$ARGV[0] = $id;
print "<br> Doing bug $id\n";
ProcessOneBug($ARGV[0]);
if (detaint_natural($id)) {
print "<br> Doing bug $id\n";
ProcessOneBug($id);
}
}
} else {
my $bugnum;
......
showdependencygraph.cgi
View file @ 9042b58f
......@@ -29,7 +29,9 @@ ConnectToDatabase();
quietly_check_login();
$::usergroupset = $::usergroupset; # More warning suppression silliness.
# More warning suppression silliness.
$::userid = $::userid;
$::usergroupset = $::usergroupset;
######################################################################
# Begin Data/Security Validation
......@@ -122,7 +124,9 @@ node [URL="${urlbase}show_bug.cgi?id=\\N", style=filled, color=lightgrey]
my $summary = "";
my $stat;
if ($::FORM{'showsummary'}) {
SendSQL("select bug_status, short_desc from bugs where bug_id = $k and bugs.groupset & $::usergroupset = bugs.groupset");
SendSQL(SelectVisible("select bug_status, short_desc from bugs where bug_id = $k",
$::userid,
$::usergroupset));
($stat, $summary) = (FetchSQLData());
$stat = "NEW" if !defined $stat;
$summary = "" if !defined $summary;
......
showdependencytree.cgi
View file @ 9042b58f
......@@ -34,7 +34,9 @@ ConnectToDatabase();
quietly_check_login();
$::usergroupset = $::usergroupset; # More warning suppression silliness.
# More warning suppression silliness.
$::userid = $::userid;
$::usergroupset = $::usergroupset;
######################################################################
# Begin Data/Security Validation
......@@ -128,10 +130,10 @@ sub DumpKids {
my ($bugid, $stat, $milestone) = ("", "", "");
my ($userid, $short_desc) = ("", "");
if (Param('usetargetmilestone')) {
SendSQL("select bug_id, bug_status, target_milestone, assigned_to, short_desc from bugs where bug_id = $kid and bugs.groupset & $::usergroupset = bugs.groupset");
SendSQL(SelectVisible("select bugs.bug_id, bug_status, target_milestone, assigned_to, short_desc from bugs where bugs.bug_id = $kid", $::userid, $::usergroupset));
($bugid, $stat, $milestone, $userid, $short_desc) = (FetchSQLData());
} else {
SendSQL("select bug_id, bug_status, assigned_to, short_desc from bugs where bug_id = $kid and bugs.groupset & $::usergroupset = bugs.groupset");
SendSQL(SelectVisible("select bugs.bug_id, bug_status, assigned_to, short_desc from bugs where bugs.bug_id = $kid", $::userid, $::usergroupset));
($bugid, $stat, $userid, $short_desc) = (FetchSQLData());
}
......
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