Repository navigation
🛡️ Sentinel: [HIGH] Throttle current-password guesses on /account/password - #136
Merged
Merged
Conversation
The sign-in form has throttled wrong-password guesses since local accounts existed; the password-change form had no such limit. That left a stolen session as an unlimited guess at the current password, and a correct one is what lets an attacker set a new password and lock the owner out of their own account. The limiter is the same one the sign-in form uses, with distinct key prefixes so a password-change bucket does not also lock legitimate login attempts from the same address or account. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011MNNzqJv2o2hxYWojiGqSs
jnnngs
marked this pull request as ready for review
November 6, 2026 19:36
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or 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
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.
🚨 Severity
High — session-theft escalation to account takeover.
💡 Vulnerability
/account/passwordre-authenticates the caller by hashing a submitted current password, but had no rate limit on failures. The login form has hadloginLimitersince local accounts existed; the password-change form does not share it. A session held by somebody who should not have it (device left unlocked, exfiltrated cookie, shared browser) was therefore an unlimited oracle for the account's current password.🎯 Impact
Once the current password is guessed, the attacker sets a new one.
SetPassworddeletes every other live session for the account, so the rightful owner is signed out of their own console and locked out until an administrator resets it — if there is a second administrator at all. On a single-admin deployment this is a full account loss.🔧 Fix
doChangePasswordnow asks the sharedloginLimiterbefore verifying the current password:change:user:<lower(username)>andchange:ip:<client>, distinct prefixes from the sign-in limiter so a password-change bucket cannot also lock legitimate login attempts for the same account or IP.RecordFailureon a wrong current password (same semantics asdoLogin).Resetonce the change succeeds, so legitimate re-changes are never blocked by old failures.429 Too Many Requests, matching the login form's shape, and the refusal is logged.No behavior change for
AUTH_MODE=none(the whole/account/passwordroute is gated onseparatesUsers(), which is false there). The default open-API deployment is untouched.✅ Verification
gofmt -l authlogin.go auth_test.go— clean.go vet ./...— clean.go test -short ./...— all pass.TestChangePasswordIsThrottledOnWrongCurrentPasswordexercises the 429 afterloginFreeAttemptswrong guesses, and that a correct password is also refused while the lockout stands.Diff: 16 production lines, 34 test lines (50 total).
Generated by Claude Code