Prevent authorized access to hashed user password, cachepwd, salt, and sessionid [SEC-3837] #15678
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… [SEC-3837] This addresses a variety of different ways a manager user with permission to manage users might be able to access hashed passwords, the cachepwd, or a users' last sessionid. Follows from report by Solar Security who reported it was included in the security/group/user/getList response, after which I did some more searching and found a few more places that could return some of these. This is not super interesting from a security perspective, as any user that has permission to manage users could change their password and just take over an account that way. But as these fields are not supposed to be readable, it's good to lock 'm down anyway.
Mark-H
added
area-security
urgent
Mark-H
added a commit
to Mark-H/revolution
that referenced
this pull request
Oct 19, 2021
|
Ported to 3.x in #15843 |
Mark-H
added a commit
to Mark-H/revolution
that referenced
this pull request
Oct 20, 2021
opengeek
added a commit
that referenced
this pull request
Oct 20, 2021
* Port security fixes from #15720 (several reflected XSS vulnerabilities) to 3.x * Port #15678 to 3.x: prevent unauthorized access to user information * Port #15643 to 3.x: prevent editing or removing of files with an extension that's not allowed * Port #15280 to 3.x: prevent stored XSS in resource list tv * Port #15273 to 3.x: fix various stored XSS vulnerabilities related to users and the media browser * Fix accidental merge artifect * grunt build Co-authored-by: Jason Coward <jason@opengeek.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does it do?
This addresses a variety of different ways a manager user with permission to manage users might be able to access hashed passwords, the cachepwd, or a users' last sessionid. Follows from report by Solar Security who reported it was included in the security/group/user/getList response, after which I did some more searching and found a few more places that could return some of these.
Why is it needed?
This is not super interesting from a security perspective, as passwords are hashed and any user that has permission to manage users could change their password and just take over an account that way. But as these fields are not supposed to be readable, good to lock 'm down anyway.
How to test
Perform the actions and confirm there is no sensitive data in the raw response.
Related issue(s)/PR(s)
https://community.modx.com/t/new-security-issues-reported-all-in-one-report/3837