Bug 438946 - Need to audit all code that enables privacy in Bugzilla to assure proper operation
Need to audit all code that enables privacy in Bugzilla to assure proper oper...
Status: CLOSED NEXTRELEASE
Product: Bugzilla
Classification: Community
Component: User Accounts (Show other bugs)
3.2
All Linux
low Severity low (vote)
: ---
: ---
Assigned To: Tony Fu
:
Depends On: 406471
Blocks: RHBZ30UpgradeTracker
  Show dependency treegraph
 
Reported: 2008-03-25 23:46 EDT by David Lawrence
Modified: 2013-06-24 00:16 EDT (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-05-27 23:48:59 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
OR based group checking in get_selectable_products() and get_enterable_products() (1.72 KB, patch)
2008-04-17 02:41 EDT, Tony Fu
dkl: review+
Details | Diff

  None (edit)
Description David Lawrence 2008-03-25 23:46:19 EDT
We need to look at all functions in Bugzilla that are used to block users from
seeing data that they are not allowed to assure that the code is performing
properly.

The following functions are used to determine permissions to view data:

Bugzilla::Search - wrapper around search SQL that blocks bugs based on permissions.

Bugzilla::User::can_see_user - Determines if user can see info about other user.

Bugzilla::User::can_edit_product - Determine whether user can edit bugs in a
specific product.

Bugzilla::User::can_see_bug - Determines whether user can see a particular bug
report.

Bugzilla::User::can_see_product - Determines whether user can query for bugs in
a specific product. Calls Bugzilla::User::get_selectable_products.

Bugzilla::User::get_selectable_products - Returns a list of products that a user
has permission to query against.

Bugzilla::User::can_enter_product - Determines whether a user can enter a bug
against a specific product. Calls Bugzilla::User::get_enterable_products().

Bugzilla::User::get_enterable_products - Returns list of products that a user
can enter bugs against.

Bugzilla::User::get_accessible_products - get_selectable_products and
get_enterable_products

Bugzilla::User::can_admin_product - Determines if user can edit attributes of a
product

Bugzilla::User::can_set_flag - Can the user set a given flag (+ -)

Bugzilla::User::can_request_flag - Can the user request a given flag (?)

Bugzilla::User::can_see_flag - Can the user even see the flag

I am sure there are more. Please add more to the list if you see any that were
missed.
Comment 1 David Lawrence 2008-04-01 10:47:48 EDT
Tony, now that fix for bug 437969 has been committed, can you take a look at
some of these functions to make sure that they are checking for proper group
permissions in the same fashion?

Thanks
Dave
Comment 2 Kevin Baker 2008-04-14 22:56:37 EDT
Tony, please review the sql for OR group checking.
Comment 3 Kevin Baker 2008-04-14 22:57:04 EDT
needed for Milestone 3. 
Comment 4 Tony Fu 2008-04-17 02:39:09 EDT
Dave,

I have reviewed all sql queries related to user, bug, product privacy.  All
functions look good, except the or base group check queries in
get_select_products and get_enterable_products.

I have attached a patch to fix them.  Please review the patch.


Thanks,
Tony
Comment 5 Tony Fu 2008-04-17 02:41:22 EDT
Created attachment 302713 [details]
OR based group checking in get_selectable_products() and get_enterable_products()
Comment 6 David Lawrence 2008-04-17 18:11:58 EDT
Comment on attachment 302713 [details]
OR based group checking in get_selectable_products() and get_enterable_products()

>+            $query .= "  WHERE group_id IN (" . $self->groups_as_string . ")" .
>+                      "     OR group_control_map.group_id IS NULL";

I think we need to put parens ( ) around the OR parts like this:

	    $query .= "  WHERE (group_id IN (" . $self->groups_as_string . ")"
.
		      " 	OR group_control_map.group_id IS NULL) ";

cause we are looking for either one to be true for that one part of the SQL. If
this seems
right and still works with your testing, then feel free to review=dkl+ and
check it in.

Thanks
Dave
Comment 7 Tony Fu 2008-04-24 02:31:04 EDT
(In reply to comment #6)
> (From update of attachment 302713 [details] [edit])
> >+            $query .= "  WHERE group_id IN (" . $self->groups_as_string . ")" .
> >+                      "     OR group_control_map.group_id IS NULL";
> 
> I think we need to put parens ( ) around the OR parts like this:
> 
> 	    $query .= "  WHERE (group_id IN (" . $self->groups_as_string . ")"
> .
> 		      " 	OR group_control_map.group_id IS NULL) ";
> 
> cause we are looking for either one to be true for that one part of the SQL. If
> this seems
> right and still works with your testing, then feel free to review=dkl+ and
> check it in.
> 
> Thanks
> Dave
> 

Dave,

I have added extra parents according to your comments and committed it into cvs
repo.


Thanks,
Tony

Note You need to log in before you can comment on or make changes to this bug.