PalisadoesFoundation / talawa-api

API Backend for the Talawa Mobile App. Click on the link below to see our documentation
https://docs.talawa.io/
GNU General Public License v3.0
231 stars 831 forks source link

Security Vulnerability: IDOR (Insecure Direct Object Reference) allows Unauthorized Profile Editing #2131

Closed krishna619 closed 1 month ago

krishna619 commented 8 months ago

Describe the bug In the Talawa admin portal, an admin can edit other users' profiles, including those of super admins, by intercepting and manipulating the edit profile request.

To Reproduce Steps to reproduce the behavior:

  1. log in as an admin to the Talawa admin portal.
  2. Navigate to the profile edit page (/member/).
  3. Intercept the profile edit request using a proxy tool.
  4. Change the ID in the request payload to that of a super admin (e.g., from 66378abd85008f171cf2990d to 64378abd85008f171cf2990d).
  5. Modify the email address in the request payload and submit the request.

Key Points 1) If I change the email address to testhacked@example.com and log in via that, It still shows the unchanged email address testsuperadmin@example.com but allows me to log in via the changed one.

2) Now even if I try to change the email to some random abc@example.com it shows a success status on the front end but it does not allow me to log in with the new one (User not found).

https://github.com/PalisadoesFoundation/talawa-api/assets/52276473/f583e4fc-d8b8-4adf-a619-69ae2fc44df5

Additional details Add any other context or screenshots about the feature request here.

Potential internship candidates Please read this if you are planning to apply for a Palisadoes Foundation internship https://github.com/PalisadoesFoundation/talawa/issues/359

krishna619 commented 8 months ago

@palisadoes I believe this issue is to be addressed from the front end as well. What are your thoughts?

palisadoes commented 8 months ago

Admins in a specific role should be able to edit the profiles of people at or below their level. Therefore:

  1. SuperAdmins can edit other SuperAdmin and Admin profiles. User/Member profiles too
  2. Admins can edit other Admin profiles. User/Member profiles too. They cannot edit SuperAdmin profiles.
  3. Users/Members must only be able to edit their own profiles

This needs to be enforced in the API first

krishna619 commented 8 months ago

@palisadoes I see, apart from it

  1. we need to handle concurrent sessions what if Super Admin is editing let's say admin A's profile and admin A is editing his/her profile at that point of time?
  2. Should we also focus on a fallback or recovery mechanism for users who lose access to their accounts due to unauthorized profile changes?
  3. can we also maintain an audit log that records when a user's profile is edited, by whom, and what changes were made?
palisadoes commented 8 months ago

Create issues as required

krishna619 commented 8 months ago

@palisadoes https://github.com/PalisadoesFoundation/talawa-api/issues/2147 done

github-actions[bot] commented 7 months ago

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

hg6658 commented 7 months ago

Can we encrypt the URL parameters also using strong cryptographically tokens which references models?

krishna619 commented 7 months ago

Encrypting won't help, anyone with the encrypted payload (visible in the request param) can still make edits. This needs auth implementation.

pranshugupta54 commented 7 months ago

@krishna619, you logged in the SuperAdmin account by changing their email id but how do u password for that superadmin account ?🤔

pranshugupta54 commented 7 months ago

I don't think there's any extra thing what interceptor does. It's just changing the request and response from API, otherwise every permissions is being checked and managed by API itself. If you're able to see superadmin details just being an Admin, then it's only bcuz User Query doesn't require role permissions. That's an expected flow and doesn't look like a bug. It might look like a very big issue from your video but that's only the frontend being manipulated due to usage of localStorage. This doesn't make changes to database which is being access via API.

If standalone API is working properly (use Apollo server to test it), then it's 100% secure even if you can manipulate frontend via interceptors or anything.

krishna619 commented 7 months ago

@pranshugupta54 once i changed the email address to testhacked@example.com I can anyways get a new password by forgetting the password, where the password would be sent to testhacked@example.com and hence i can login as super admin.

pranshugupta54 commented 7 months ago

@krishna619, did it change the email in database? I don't think so? It's just frontend part interceptor doing it.

krishna619 commented 7 months ago

How did it make him log in if it didn't save in the backend? Will look into it more thoroughly.

pranshugupta54 commented 7 months ago

Most probably the interceptor is just saving it on the frontend

github-actions[bot] commented 7 months ago

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

github-actions[bot] commented 1 month ago

This issue did not get any activity in the past 180 days and thus has been closed. Please check if the newest release or develop branch has it fixed. Please, create a new issue if the issue is not fixed.