зеркало из https://github.com/mozilla/pjs.git
Bug 293159: [SECURITY] Anyone can change flags and access bug summaries due to a bad check in Flag::validate() and Flag::modify()
Patch By Frederic Buclin <LpSolit@gmail.com> r=myk, a=justdave
This commit is contained in:
mkanat%kerio.com
Родитель
7d37188dd3
Коммит
22ec74ac9a
|
|
@ -235,17 +235,47 @@ Validates fields containing flag modifications.
|
|||
=cut
|
||||
|
||||
sub validate {
|
||||
my ($cgi, $bug_id, $attach_id) = @_;
|
||||
|
||||
my $user = Bugzilla->user;
|
||||
my ($cgi, $bug_id) = @_;
|
||||
|
||||
my $dbh = Bugzilla->dbh;
|
||||
|
||||
# Get a list of flags to validate. Uses the "map" function
|
||||
# to extract flag IDs from form field names by matching fields
|
||||
# whose name looks like "flag-nnn", where "nnn" is the ID,
|
||||
# and returning just the ID portion of matching field names.
|
||||
my @ids = map(/^flag-(\d+)$/ ? $1 : (), $cgi->param());
|
||||
|
||||
foreach my $id (@ids)
|
||||
{
|
||||
|
||||
return unless scalar(@ids);
|
||||
|
||||
# No flag reference should exist when changing several bugs at once.
|
||||
ThrowCodeError("flags_not_available", { type => 'b' }) unless $bug_id;
|
||||
|
||||
# No reference to existing flags should exist when creating a new
|
||||
# attachment.
|
||||
if ($attach_id && ($attach_id < 0)) {
|
||||
ThrowCodeError("flags_not_available", { type => 'a' });
|
||||
}
|
||||
|
||||
# Make sure all flags belong to the bug/attachment they pretend to be.
|
||||
my $field = ($attach_id) ? "attach_id" : "bug_id";
|
||||
my $field_id = $attach_id || $bug_id;
|
||||
my $not = ($attach_id) ? "" : "NOT";
|
||||
|
||||
my $invalid_data =
|
||||
$dbh->selectrow_array("SELECT 1 FROM flags
|
||||
WHERE id IN (" . join(',', @ids) . ")
|
||||
AND ($field != ? OR attach_id IS $not NULL) " .
|
||||
$dbh->sql_limit(1),
|
||||
undef, $field_id);
|
||||
|
||||
if ($invalid_data) {
|
||||
ThrowCodeError("invalid_flag_association",
|
||||
{ bug_id => $bug_id,
|
||||
attach_id => $attach_id });
|
||||
}
|
||||
|
||||
foreach my $id (@ids) {
|
||||
my $status = $cgi->param("flag-$id");
|
||||
|
||||
# Make sure the flag exists.
|
||||
|
|
@ -269,48 +299,60 @@ sub validate {
|
|||
ThrowCodeError("flag_status_invalid",
|
||||
{ id => $id, status => $status });
|
||||
}
|
||||
|
||||
|
||||
# Make sure the user didn't specify a requestee unless the flag
|
||||
# is specifically requestable. If the requestee was set before
|
||||
# the flag became specifically unrequestable, leave it as is.
|
||||
my $old_requestee =
|
||||
$flag->{'requestee'} ? $flag->{'requestee'}->login : '';
|
||||
my $new_requestee = trim($cgi->param("requestee-$id") || '');
|
||||
|
||||
if ($status eq '?'
|
||||
&& !$flag->{type}->{is_requesteeble}
|
||||
&& $new_requestee
|
||||
&& ($new_requestee ne $old_requestee))
|
||||
{
|
||||
ThrowCodeError("flag_requestee_disabled",
|
||||
{ name => $flag->{type}->{name} });
|
||||
}
|
||||
|
||||
# Make sure the requestee is authorized to access the bug.
|
||||
# (and attachment, if this installation is using the "insider group"
|
||||
# feature and the attachment is marked private).
|
||||
if ($status eq '?'
|
||||
&& $flag->{type}->{is_requesteeble}
|
||||
&& trim($cgi->param("requestee-$id")))
|
||||
&& $new_requestee
|
||||
&& ($old_requestee ne $new_requestee))
|
||||
{
|
||||
my $requestee_email = trim($cgi->param("requestee-$id"));
|
||||
my $old_requestee =
|
||||
$flag->{'requestee'} ? $flag->{'requestee'}->login : '';
|
||||
# We know the requestee exists because we ran
|
||||
# Bugzilla::User::match_field before getting here.
|
||||
my $requestee = Bugzilla::User->new_from_login($new_requestee);
|
||||
|
||||
if ($old_requestee ne $requestee_email) {
|
||||
# We know the requestee exists because we ran
|
||||
# Bugzilla::User::match_field before getting here.
|
||||
my $requestee = Bugzilla::User->new_from_login($requestee_email);
|
||||
# Throw an error if the user can't see the bug.
|
||||
# Note that if permissions on this bug are changed,
|
||||
# can_see_bug() will refer to old settings.
|
||||
if (!$requestee->can_see_bug($bug_id)) {
|
||||
ThrowUserError("flag_requestee_unauthorized",
|
||||
{ flag_type => $flag->{'type'},
|
||||
requestee => $requestee,
|
||||
bug_id => $bug_id,
|
||||
attach_id =>
|
||||
$flag->{target}->{attachment}->{id} });
|
||||
}
|
||||
|
||||
# Throw an error if the user can't see the bug.
|
||||
if (!$requestee->can_see_bug($bug_id))
|
||||
{
|
||||
ThrowUserError("flag_requestee_unauthorized",
|
||||
{ flag_type => $flag->{'type'},
|
||||
requestee => $requestee,
|
||||
bug_id => $bug_id,
|
||||
attach_id =>
|
||||
$flag->{target}->{attachment}->{id} });
|
||||
}
|
||||
|
||||
# Throw an error if the target is a private attachment and
|
||||
# the requestee isn't in the group of insiders who can see it.
|
||||
if ($flag->{target}->{attachment}->{exists}
|
||||
&& $cgi->param('isprivate')
|
||||
&& Param("insidergroup")
|
||||
&& !$requestee->in_group(Param("insidergroup")))
|
||||
{
|
||||
ThrowUserError("flag_requestee_unauthorized_attachment",
|
||||
{ flag_type => $flag->{'type'},
|
||||
requestee => $requestee,
|
||||
bug_id => $bug_id,
|
||||
attach_id =>
|
||||
$flag->{target}->{attachment}->{id} });
|
||||
}
|
||||
# Throw an error if the target is a private attachment and
|
||||
# the requestee isn't in the group of insiders who can see it.
|
||||
if ($flag->{target}->{attachment}->{exists}
|
||||
&& $cgi->param('isprivate')
|
||||
&& Param("insidergroup")
|
||||
&& !$requestee->in_group(Param("insidergroup")))
|
||||
{
|
||||
ThrowUserError("flag_requestee_unauthorized_attachment",
|
||||
{ flag_type => $flag->{'type'},
|
||||
requestee => $requestee,
|
||||
bug_id => $bug_id,
|
||||
attach_id =>
|
||||
$flag->{target}->{attachment}->{id} });
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -325,13 +325,32 @@ and returning just the ID portion of matching field names.
|
|||
=cut
|
||||
|
||||
sub validate {
|
||||
my $user = Bugzilla->user;
|
||||
my ($cgi, $bug_id, $attach_id) = @_;
|
||||
|
||||
|
||||
my $user = Bugzilla->user;
|
||||
my $dbh = Bugzilla->dbh;
|
||||
|
||||
my @ids = map(/^flag_type-(\d+)$/ ? $1 : (), $cgi->param());
|
||||
|
||||
foreach my $id (@ids)
|
||||
{
|
||||
return unless scalar(@ids);
|
||||
|
||||
# No flag reference should exist when changing several bugs at once.
|
||||
ThrowCodeError("flags_not_available", { type => 'b' }) unless $bug_id;
|
||||
|
||||
# We don't check that these flag types are valid for
|
||||
# this bug/attachment. This check will be done later when
|
||||
# processing new flags, see Flag::FormToNewFlags().
|
||||
|
||||
# All flag types have to be active
|
||||
my $inactive_flagtypes =
|
||||
$dbh->selectrow_array("SELECT 1 FROM flagtypes
|
||||
WHERE id IN (" . join(',', @ids) . ")
|
||||
AND is_active = 0 " .
|
||||
$dbh->sql_limit(1));
|
||||
|
||||
ThrowCodeError("flag_type_inactive") if $inactive_flagtypes;
|
||||
|
||||
foreach my $id (@ids) {
|
||||
my $status = $cgi->param("flag_type-$id");
|
||||
|
||||
# Don't bother validating types the user didn't touch.
|
||||
|
|
@ -353,22 +372,31 @@ sub validate {
|
|||
{ id => $id , status => $status });
|
||||
}
|
||||
|
||||
# Make sure the user didn't specify a requestee unless the flag
|
||||
# is specifically requestable.
|
||||
my $new_requestee = trim($cgi->param("requestee_type-$id") || '');
|
||||
|
||||
if ($status eq '?'
|
||||
&& !$flag_type->{is_requesteeble}
|
||||
&& $new_requestee)
|
||||
{
|
||||
ThrowCodeError("flag_requestee_disabled",
|
||||
{ name => $flag_type->{name} });
|
||||
}
|
||||
|
||||
# Make sure the requestee is authorized to access the bug
|
||||
# (and attachment, if this installation is using the "insider group"
|
||||
# feature and the attachment is marked private).
|
||||
if ($status eq '?'
|
||||
&& $flag_type->{is_requesteeble}
|
||||
&& trim($cgi->param("requestee_type-$id")))
|
||||
&& $new_requestee)
|
||||
{
|
||||
my $requestee_email = trim($cgi->param("requestee_type-$id"));
|
||||
|
||||
# We know the requestee exists because we ran
|
||||
# Bugzilla::User::match_field before getting here.
|
||||
my $requestee = Bugzilla::User->new_from_login($requestee_email);
|
||||
my $requestee = Bugzilla::User->new_from_login($new_requestee);
|
||||
|
||||
# Throw an error if the user can't see the bug.
|
||||
if (!$requestee->can_see_bug($bug_id))
|
||||
{
|
||||
if (!$requestee->can_see_bug($bug_id)) {
|
||||
ThrowUserError("flag_requestee_unauthorized",
|
||||
{ flag_type => $flag_type,
|
||||
requestee => $requestee,
|
||||
|
|
|
|||
|
|
@ -913,8 +913,11 @@ sub insert
|
|||
$vars->{'message'} = 'user_match_multiple';
|
||||
}
|
||||
|
||||
Bugzilla::Flag::validate($cgi, $bugid);
|
||||
Bugzilla::FlagType::validate($cgi, $bugid, $cgi->param('id'));
|
||||
# Flag::validate() should not detect any reference to existing
|
||||
# flags when creating a new attachment. Setting the third param
|
||||
# to -1 will force this function to check this point.
|
||||
Bugzilla::Flag::validate($cgi, $bugid, -1);
|
||||
Bugzilla::FlagType::validate($cgi, $bugid);
|
||||
|
||||
# Escape characters in strings that will be used in SQL statements.
|
||||
my $sql_filename = SqlQuote($filename);
|
||||
|
|
@ -1148,7 +1151,7 @@ sub update
|
|||
Bugzilla::User::match_field($cgi, {
|
||||
'^requestee(_type)?-(\d+)$' => { 'type' => 'single' }
|
||||
});
|
||||
Bugzilla::Flag::validate($cgi, $bugid);
|
||||
Bugzilla::Flag::validate($cgi, $bugid, $attach_id);
|
||||
Bugzilla::FlagType::validate($cgi, $bugid, $attach_id);
|
||||
|
||||
# Lock database tables in preparation for updating the attachment.
|
||||
|
|
|
|||
|
|
@ -165,12 +165,11 @@ foreach my $field ("dependson", "blocked") {
|
|||
'assigned_to' => { 'type' => 'single' },
|
||||
'^requestee(_type)?-(\d+)$' => { 'type' => 'single' },
|
||||
});
|
||||
# Validate flags, but only if the user is changing a single bug,
|
||||
# since the multi-change form doesn't include flag changes.
|
||||
if (defined $cgi->param('id')) {
|
||||
Bugzilla::Flag::validate($cgi, $cgi->param('id'));
|
||||
Bugzilla::FlagType::validate($cgi, $cgi->param('id'));
|
||||
}
|
||||
|
||||
# Validate flags in all cases. validate() should not detect any
|
||||
# reference to flags if $cgi->param('id') is undefined.
|
||||
Bugzilla::Flag::validate($cgi, $cgi->param('id'));
|
||||
Bugzilla::FlagType::validate($cgi, $cgi->param('id'));
|
||||
|
||||
######################################################################
|
||||
# End Data/Security Validation
|
||||
|
|
|
|||
|
|
@ -135,6 +135,15 @@
|
|||
[% title = "Invalid Dimensions" %]
|
||||
The width or height specified is not a positive integer.
|
||||
|
||||
[% ELSIF error == "invalid_flag_association" %]
|
||||
[% title = "Invalid Flag Association" %]
|
||||
Some flags do not belong to
|
||||
[% IF attach_id %]
|
||||
attachment [% attach_id FILTER html %].
|
||||
[% ELSE %]
|
||||
[%+ terms.bug %] [%+ bug_id FILTER html %].
|
||||
[% END %]
|
||||
|
||||
[% ELSIF error == "invalid_isactive_flag" %]
|
||||
[% title = "Invalid isactive flag" %]
|
||||
The active flag was improperly set. There may be
|
||||
|
|
@ -153,6 +162,20 @@
|
|||
|
||||
[% ELSIF error == "flag_nonexistent" %]
|
||||
There is no flag with ID #[% id FILTER html %].
|
||||
|
||||
[% ELSIF error == "flags_not_available" %]
|
||||
[% title = "Flag Editing not Allowed" %]
|
||||
[% IF type == "b" %]
|
||||
Flags cannot be set or changed when
|
||||
changing several [% terms.bugs %] at once.
|
||||
[% ELSE %]
|
||||
References to existing flags when creating
|
||||
a new attachment are invalid.
|
||||
[% END %]
|
||||
|
||||
[% ELSIF error == "flag_requestee_disabled" %]
|
||||
[% title = "Flag not Specifically Requestable" %]
|
||||
The flag <em>[% name FILTER html %]</em> is not specifically requestable.
|
||||
|
||||
[% ELSIF error == "flag_status_invalid" %]
|
||||
The flag status <em>[% status FILTER html %]</em>
|
||||
|
|
@ -172,6 +195,10 @@
|
|||
The flag type ID <em>[% id FILTER html %]</em> is not
|
||||
a positive integer.
|
||||
|
||||
[% ELSIF error == "flag_type_inactive" %]
|
||||
[% title = "Inactive Flag Types" %]
|
||||
Some flag types are inactive and cannot be used to create new flags.
|
||||
|
||||
[% ELSIF error == "flag_type_nonexistent" %]
|
||||
There is no flag type with the ID <em>[% id FILTER html %]</em>.
|
||||
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче