Skip to content

Require the current password to change the password - #848

Open
yuzi-co wants to merge 1 commit into
sipeed:mainfrom
yuzi-co:security/password-change-requires-current
Open

Require the current password to change the password#848
yuzi-co wants to merge 1 commit into
sipeed:mainfrom
yuzi-co:security/password-change-requires-current

Conversation

@yuzi-co

@yuzi-co yuzi-co commented Aug 1, 2026

Copy link
Copy Markdown

ChangePassword rewrites the account file and the Linux root password without asking for the current one.

Why that matters

Any request that arrives with a valid session changes the password. So a session cookie lifted from a logged-in browser, or a forged cross-site request, is not just access — it is a permanent takeover. The attacker sets a password of their choosing and the operator is locked out of their own device.

This compounds with the missing origin check (#847), but it is worth fixing on its own: a stolen session should not be upgradable into permanent ownership.

What this does

The current password is required and verified before either write.

The one exception is a device with no account file, which is therefore still on the documented admin/admin default. There is no secret to prove there, and requiring one would lock people out of first-time setup.

Only a missing account file counts as that exception. Any other stat error leaves the answer unknown, and an unknown answer keeps the check in force rather than waiving it.

The wrong-password path sleeps for two seconds, matching what Login already does for the same case.

AccountFile becomes a var so the tests can point it at a temp directory.

Tests

service/auth/password_test.go covers: correct current password succeeds, wrong one is refused, missing one is refused, an unreadable account file still demands the current password, and the unconfigured-device path still works.

Frontend changes are the matching field on the change-password form plus one English string; other locales fall back until translated.

Verified with go build, go vet, go test, and GOOS=linux GOARCH=riscv64 go build.


Updated: the two file-permission fixes that were originally bundled here have been split into their own PR. They are unrelated to this change and much easier to review on their own. This PR is now only the password check.

@yuzi-co
yuzi-co force-pushed the security/password-change-requires-current branch 2 times, most recently from e6a937d to dfa7cc4 Compare August 3, 2026 06:07
ChangePassword rewrote the account file and the Linux root password without
asking for the current one. A single forged request, or a session cookie
lifted from a logged-in browser, was therefore a permanent takeover: the
attacker sets a password of their choosing and the operator is locked out of
their own device.

The current password is now required and verified before either write. The
one exception is a device that has no account file yet and is therefore still
on the documented admin/admin default, where there is no secret to prove.

Only a missing account file counts as that exception. Any other stat error
leaves the answer unknown, and an unknown answer keeps the check in force
rather than waiving it.

The wrong-password path sleeps for two seconds, which is what Login already
does for the same case.

AccountFile becomes a var so the tests can redirect it to a temporary
directory.
@yuzi-co
yuzi-co force-pushed the security/password-change-requires-current branch from dfa7cc4 to 7ac36a6 Compare August 13, 2026 17:48
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.

1 participant