]> git.ipfire.org Git - thirdparty/bugzilla.git/commitdiff
Bug 293159: [SECURITY] Anyone can change flags and access bug summaries due to a...
authormkanat%kerio.com <>
Fri, 8 Jul 2005 12:30:58 +0000 (12:30 +0000)
committermkanat%kerio.com <>
Fri, 8 Jul 2005 12:30:58 +0000 (12:30 +0000)
Patch By Frederic Buclin <LpSolit@gmail.com> r=myk, a=justdave

Bugzilla/Flag.pm
Bugzilla/FlagType.pm
attachment.cgi
process_bug.cgi
template/en/default/global/code-error.html.tmpl

index 37de4a72025530070039903689adb0771ac973a6..1b00676742ccbfa3ac38bd5c3d174dca52e3f347 100644 (file)
@@ -139,17 +139,39 @@ sub count {
 
 sub validate {
     # Validates fields containing flag modifications.
-
-    my ($data, $bug_id) = @_;
+    my ($data, $bug_id, $attach_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 : (), keys %$data);
-  
-    foreach my $id (@ids)
-    {
+
+    return unless scalar(@ids);
+
+    # No flag reference should exist when changing several bugs at once.
+    ThrowCodeError("flags_not_available") unless $bug_id;
+
+    # 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)
+                               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 = $data->{"flag-$id"};
         
         # Make sure the flag exists.
@@ -173,45 +195,59 @@ 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($data->{"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($data->{"requestee-$id"}))
+            && $new_requestee
+            && ($old_requestee ne $new_requestee))
         {
-            my $requestee_email = trim($data->{"requestee-$id"});
-            if ($requestee_email ne $flag->{'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.
-                if (!&::CanSeeBug($bug_id, $requestee->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}
-                    && $data->{'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} });
-                }
+            # We know the requestee exists because we ran
+            # Bugzilla::User::match_field before getting here.
+            my $requestee = Bugzilla::User->new_from_login($new_requestee);
+
+            # Throw an error if the user can't see the bug.
+            if (!&::CanSeeBug($bug_id, $requestee->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}
+                && $data->{'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} });
             }
         }
     }
index b28a40c143c54dcd77e72ae0fcd53a04106a537d..58e7eb0bdeeef2e70f025718d8ef6fbd37e6c1aa 100644 (file)
@@ -194,15 +194,28 @@ sub count {
 
 sub validate {
     my ($data, $bug_id, $attach_id) = @_;
-  
+    my $dbh = Bugzilla->dbh;
+
     # Get a list of flag types to validate.  Uses the "map" function
     # to extract flag type IDs from form field names by matching columns
     # whose name looks like "flag_type-nnn", where "nnn" is the ID,
     # and returning just the ID portion of matching field names.
     my @ids = map(/^flag_type-(\d+)$/ ? $1 : (), keys %$data);
-  
-    foreach my $id (@ids)
-    {
+
+    return unless scalar(@ids);
+
+    # No flag reference should exist when changing several bugs at once.
+    ThrowCodeError("flags_not_available") unless $bug_id;
+
+    # 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 LIMIT 1");
+
+    ThrowCodeError("flag_type_inactive") if $inactive_flagtypes;
+
+    foreach my $id (@ids) {
         my $status = $data->{"flag_type-$id"};
         
         # Don't bother validating types the user didn't touch.
@@ -223,23 +236,32 @@ 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.
+        my $new_requestee = trim($data->{"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($data->{"requestee_type-$id"}))
+            && $new_requestee)
         {
-            my $requestee_email = trim($data->{"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 (!&::CanSeeBug($bug_id, $requestee->id))
-            {
+            if (!&::CanSeeBug($bug_id, $requestee->id)) {
                 ThrowUserError("flag_requestee_unauthorized",
                                { flag_type => $flag_type,
                                  requestee => $requestee,
index ccd55fce5cb4e31754663c21bf3671f04f786b4e..6b863d0ea4d514e853712584ff13491580c351ce 100755 (executable)
@@ -146,7 +146,7 @@ elsif ($action eq "update")
   # in the requestee fields are legitimate user email addresses.
   Bugzilla::User::match_field({ '^requestee(_type)?-(\d+)$' => 
                                     { 'type' => 'single' } });
-  Bugzilla::Flag::validate(\%::FORM, $bugid);
+  Bugzilla::Flag::validate(\%::FORM, $bugid, $::FORM{'id'});
   Bugzilla::FlagType::validate(\%::FORM, $bugid, $::FORM{'id'});
   
   update();
index c78673af785b5210bdd3bf0cb8a3bfc20bc20b7f..cf1046b69c485747a59157975640d174750e24ac 100755 (executable)
@@ -145,12 +145,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 $::FORM{'id'}) {
-    Bugzilla::Flag::validate(\%::FORM, $::FORM{'id'});
-    Bugzilla::FlagType::validate(\%::FORM, $::FORM{'id'});
-}
+
+# Validate flags in all cases. validate() should not detect any
+# reference to flags if $::FORM{'id'} is undefined.
+Bugzilla::Flag::validate(\%::FORM, $::FORM{'id'});
+Bugzilla::FlagType::validate(\%::FORM, $::FORM{'id'});
 
 ######################################################################
 # End Data/Security Validation
index 5053f0b2fe6ac79ac6ca966b533f2c11b720ed5b..063c82dd010154f7f78cf52b748edc8a942297db 100644 (file)
     is attached to [% terms.bug %] [%+ attach_bug_id FILTER html %], 
     but you tried to flag it as obsolete while creating a new attachment to 
     [% terms.bug %] [%+ my_bug_id FILTER html %].
-        
+
+  [% 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 == "flag_nonexistent" %]
     There is no flag with ID #[% id FILTER html %].
-  
+
+  [% ELSIF error == "flags_not_available" %]
+    [% title = "Flag Editing not Allowed" %]
+    Flags cannot be set or changed when
+    changing several [% terms.bugs %] at once.
+
+  [% 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>
     [% IF id %]
     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>.