Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
krishna619 opened this issue Mar 31, 2024 · 15 comments
Labels
bug Something isn't working feature request no-issue-activity No issue activity unapproved Unapproved for Pull Request

Comments

@krishna619
Copy link

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).

video.mp4

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 PalisadoesFoundation/talawa#359

@krishna619 krishna619 added the bug Something isn't working label Mar 31, 2024
@github-actions github-actions bot added feature request unapproved Unapproved for Pull Request labels Mar 31, 2024
@krishna619
Copy link
Author

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

@palisadoes
Copy link
Contributor

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
Copy link
Author

@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
Copy link
Contributor

Create issues as required

@krishna619
Copy link
Author

@palisadoes #2147 done

Copy link

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 github-actions bot added the no-issue-activity No issue activity label Apr 14, 2024
@hg6658
Copy link

hg6658 commented Apr 21, 2024

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

@krishna619
Copy link
Author

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

@pranshugupta54
Copy link
Member

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

@pranshugupta54
Copy link
Member

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
Copy link
Author

@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
Copy link
Member

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

@krishna619
Copy link
Author

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

@pranshugupta54
Copy link
Member

Most probably the interceptor is just saving it on the frontend

Copy link

github-actions bot commented May 4, 2024

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 github-actions bot added the no-issue-activity No issue activity label May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feature request no-issue-activity No issue activity unapproved Unapproved for Pull Request
Projects
None yet
Development

No branches or pull requests

4 participants