Admidio / admidio

Admidio is a free open source user management system for websites of organizations and groups. The system has a flexible role model so that it’s possible to reflect the structure and permissions of your organization.
https://www.admidio.org
GNU General Public License v2.0
336 stars 131 forks source link

possible unauthorized access to other users profile data #354

Closed papierkorb27 closed 8 years ago

papierkorb27 commented 8 years ago

There seems to be a problem in v3.1.3 regarding access to profiles from others members even though access should be disabled:

Reproduced on http://demo.admidio.org

  1. create user1 and user2, move user1 in role1 and user2 in role2
  2. login as user2, open your profile and change the profile number in the url with any other one or from user1. Otherwiste open the calender module and a date with registration and maybe you can see other users which are not in your group, just click on the name of the user. Now you can see the profile data.
  3. The process is even possible, if you switch of the permissions of group1 and group2 at "Rollenmitgliedschaft sehen" and remove "Mitgliederlisten aller Rollen einsehen". And it still works too, if you remove the users from common used roles.
papierkorb27 commented 8 years ago

Using the code from the current git checkout of the /adm_program/system/classes/user.php and adapting to the code in v3.1.3 to line 938 seems to work:

{ // alle angemeldeten Benutzer duerfen Rollenlisten/-profile sehen return true; } if($row['rol_this_list_view'] == 1 && array_key_exists($row['rol_id'], $this->list_view_rights) && $this->list_view_rights[$row['rol_id']]) { // nur Rollenmitglieder duerfen Rollenlisten/-profile sehen return true; }

Additionally its a good idea to add the condition "AND cat_id != 3" to the prior sql query that the user cant access to other profiles from user which take part to a same presentation with registration.

Fasse commented 8 years ago

Your proposed fix worked and looks good. Thanks for the hint.

papierkorb27 commented 8 years ago

Hi fasse, wouldnt be wise to add the SQL statement "AND cat_id != 3" to line 919? Otherwise you have access to other profiles from users which take part to a same presentation with registration?

Fasse commented 8 years ago

what is cat_id 3 in your installation?

what do you mean with

presentation with registration ?

papierkorb27 commented 8 years ago

I thought entries with cat_id 3 are "date roles" in every installation?

prensentation with registration = event dates where i can register my participation

I think if you didnt restrict this query, you get access to every other profile data vom users which take part at this event date too (because you are in the same "date"-role).

papierkorb27 commented 8 years ago

Hi fasse,

have you thought about the role problem? Users can take part at the same event ("event role") but can be in different roles. at this situation they shouldnt have access to the other profiles. at this moment they have access because they are in the same event role

The proposed fix "AND cat_id != 3" didn't work, we need a check if the user is just in the same "event role" and in no other common roles, the access should be forbidden.

Fasse commented 8 years ago

I will look at this.

Fasse commented 8 years ago

@papierkorb27 this is because the event roles have the right to view profile data of other role members. If you remove this right then the users are not able to view the profile. Maybe we must add restrictive rights to event roles in future versions.

papierkorb27 commented 8 years ago

Ok, so it would be an option to disable the right "Rollenmitgliedschaft sehen" by default if a user creates a new event role? Should I create a new issue?