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

Harden security/profile/get processor #16439

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

opengeek
Copy link
Member

What does it do?

Prevents unauthorized users from accessing potentially sensitive profile data of other users.

Why is it needed?

The security/profile/get processor currently accepts an id property and looks up a user by id. It also responds with fields that could then be used to hijack another user's session. This process should not:

  • allow loading a user other than the currently authenticated user
  • return sensitive data in the response such as sessionid or the password hash

How to test

Call the security/profile/get processor when logged in to the manager and attempt to access another user's profile by id. Also, verify the response does not include password or sessionid fields even when returning your own profile.

Related issue(s)/PR(s)

2.x backport of #16437

@opengeek opengeek added bug The issue in the code or project, which should be addressed. area-security urgent The issue requires attention and has higher priority over others. labels Jun 12, 2023
@opengeek opengeek added this to the v2.8.6 milestone Jun 12, 2023
@opengeek opengeek requested a review from Mark-H as a code owner June 12, 2023 14:29
@cla-bot cla-bot bot added the cla-signed CLA confirmed for contributors to this PR. label Jun 12, 2023
@Ruslan-Aleev Ruslan-Aleev added the pr/ready-for-merging Pull request reviewed and tested and ready for merging. label Aug 7, 2023
@opengeek opengeek merged commit c9269f7 into modxcms:2.x Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-security bug The issue in the code or project, which should be addressed. cla-signed CLA confirmed for contributors to this PR. pr/ready-for-merging Pull request reviewed and tested and ready for merging. urgent The issue requires attention and has higher priority over others.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants