Skip to content

Don't send the account password hash in GetUser replies - #174

Merged
jhalter merged 1 commit into
jhalter:masterfrom
mishan:get-user-password-placeholder
Oct 10, 2026
Merged

jhalter merged 1 commit into
jhalter:masterfrom
mishan:get-user-password-placeholder

Conversation

@mishan

@mishan mishan commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

HandleGetUser puts the stored bcrypt hash in field 106. Two problems follow:

  • Saving an account can lock it out. A client that sends the field back unchanged in Set User (353) or Update User (355) gets the hash itself hashed and stored as the new password, so the account can no longer log in after a save that didn't touch the password. I ran into this from GtkHx's account editor, which now works around it.
  • It discloses hashes. Anyone with the Open User privilege can read every account's hash and crack it offline.

This replies with x when the account has a password and leaves the field out otherwise. That matches what Account.Read already sends in the user list (348), and what the original server sends. Clients that send 00 for an untouched password, the keep-password case HandleSetUser handles, behave as before.

Tests: TestHandleGetUser now covers an account with a password and one without.

HandleGetUser put the stored bcrypt hash in field 106. A client that
sends the field back unchanged in SetUser or UpdateUser has the hash
itself hashed and stored, so saving an account without touching its
password locks it out. It also hands the hashes to anyone with the
Open User privilege.

Reply with "x" when the account has a password and omit the field
otherwise, as Account.Read already does for the user list.
@jhalter
jhalter merged commit 1aab318 into jhalter:master Oct 10, 2026
2 checks passed
@mishan
mishan deleted the get-user-password-placeholder branch October 10, 2026 05:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants