talawa-api icon indicating copy to clipboard operation
talawa-api copied to clipboard

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

Open krishna619 opened this issue 10 months ago • 15 comments

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 [email protected] and log in via that, It still shows the unchanged email address [email protected] but allows me to log in via the changed one.

  2. Now even if I try to change the email to some random [email protected] 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 avatar Mar 31 '24 21:03 krishna619

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

krishna619 avatar Mar 31 '24 21:03 krishna619

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

palisadoes avatar Mar 31 '24 22:03 palisadoes

@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?

krishna619 avatar Apr 01 '24 05:04 krishna619

Create issues as required

palisadoes avatar Apr 02 '24 11:04 palisadoes

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

krishna619 avatar Apr 03 '24 05:04 krishna619

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] avatar Apr 14 '24 00:04 github-actions[bot]

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

hg6658 avatar Apr 21 '24 18:04 hg6658

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

krishna619 avatar Apr 23 '24 07:04 krishna619

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

pranshugupta54 avatar Apr 23 '24 07:04 pranshugupta54

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.

pranshugupta54 avatar Apr 23 '24 07:04 pranshugupta54

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

krishna619 avatar Apr 23 '24 08:04 krishna619

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

pranshugupta54 avatar Apr 23 '24 08:04 pranshugupta54

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

krishna619 avatar Apr 23 '24 10:04 krishna619

Most probably the interceptor is just saving it on the frontend

pranshugupta54 avatar Apr 23 '24 10:04 pranshugupta54

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] avatar May 04 '24 00:05 github-actions[bot]