Bug 6676 - Acquisition basket access control trivially by-passable
Summary: Acquisition basket access control trivially by-passable
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: 3.4
Hardware: All All
: P5 - low normal (vote)
Assignee: Bugs List
QA Contact: Bugs List
URL:
Keywords:
Depends on: 6390
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-07 11:05 UTC by Edgar Fuß
Modified: 2015-12-03 22:00 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
. (83.41 KB, image/png)
2011-08-08 21:46 UTC, Mason James
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Edgar Fuß 2011-08-07 11:05:54 UTC
Currently, while booksellers.pl tries to restrict a user's ability to view baskets, that's trivially circumvented by simply altering the basketno CGI parameter to, e.g. baket.pl.

I can think of three ways to close this security hole:

1. Check permissions in every script that deals with baskets. This would probably require an extension to C4::Auth.
2. Randomise basket numbers.
3. Add a random key to each basket that must be given as a CGI parameter (in addition to basketno) in order for a script to allow access to that basket.
Comment 1 Mason James 2011-08-08 21:46:50 UTC
Created attachment 4900 [details]
.

there are 3 user-permission settings to control access to acq-baskets

•  group_manage Manage orders & basketgroups
•  order_manage Manage orders & basket
•  order_receive Manage orders & basket


have you tested accessing a basket, with all 3 settings 'off'?
Comment 2 Edgar Fuß 2011-08-09 09:52:28 UTC
I was thinking of two members of staff (working at different branches) which both have been granted the rights to handle acquisitions. Nevertheless you may not want one branch's staff to tamper with the acquisitions of another branch.
Comment 3 Katrin Fischer 2011-08-09 10:57:06 UTC
I will try to explain the problem a bit differently:

At the moment staff members can only see their own baskets on the vendor page. And they are supposed to only take a look at their own baskets - so changing the url should not work for them. 

The fact that staff members can only see their own paskets can be a problem depending on the workflow in the specific library. I have filed bug 6390 for this problem, but as the behaviour is intended we need to find a way to make it configurable.

I think the best and most granular solution for this problem would be to add a new permission 'Manage and view all baskets'. The other permissions could be reworded too, to make it a bit more clear how they will work and affect what you can do.

I think the problem is not guessing the url of another basket, it's that the script should check if you are allowed to see the basket and change the display appropriately.

So if you are not allowed to - show a message telling so.
If you are allowed - show the basket and its contents.

I think this would be consistent with how Koha works in other places and therefore a better solution than randomizing parts of the URL.
Comment 4 Owen Leonard 2014-04-25 19:25:43 UTC
Is this bug valid in master?
Comment 5 Katrin Fischer 2015-01-06 18:03:11 UTC
Bug appears no longer valid on master.

Without the order_manage and order_manage_all permissions the vendor search shows only the vendor names, no options to create or access baskets.

Trying to reach a basket directly via URL fails - a login page is presented.

Without oder_manage_all and AcqViewBaskets set to "created or managed by staff member" an error message is shown when trying to access another person's basket:
You are not authorised to manage this basket.