Change PIN never checks the current PIN #60

New Issue

Closed

opened 2026-08-20 21:22:36 -05:00 by null · 0 comments

null commented

2026-08-20 21:22:36 -05:00

Owner

Found by tracing the app-lock flow at 93ec5b7.

What is true now. app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockSettingsScreen.kt:84-93, the CONFIRM_TO_CHANGE branch, calls viewModel.verifyCurrent(pin) { } — asynchronous, with an EMPTY callback — and then assigns mode = Mode.SET_REPLACEMENT unconditionally, outside the result. verifyCurrent (LockSettingsViewModel.kt:99-113) only sets a WRONG_PIN message; nothing gates the transition. SET_REPLACEMENT calls setPin, and AppLockRepository.setPin (core/security/.../AppLockRepository.kt:92-100) enrolls without verifying anything.

So: Change PIN, type any four digits, and you reach 'Choose a PIN' and overwrite the PIN.

What it costs. Anyone holding the phone while it is unlocked can change the app's PIN. Under this project's no-recovery policy the owner's only way back into her own history is to erase all of it. That is the exact bypass the file's own KDoc says must not be possible (LockSettingsScreen.kt:45-53, LockSettingsViewModel.kt:31-40).

What to do. Move the transition inside the verified callback, exactly as CONFIRM_TO_REMOVE (:76-82) already does — onAuthoriseChange(pin) { mode = SET_REPLACEMENT }. Add a guard at the write site too: the ViewModel refuses setPin when a PIN already exists and no successful check authorised a replacement, because the place that writes the PIN is the place that must refuse.

Traps. CONFIRM_TO_REMOVE has the same advance-before-async shape and is benign for security, but its wrong-PIN message lands on the Overview instead of the PIN screen (ConfirmPin.wrong is dead) — fix it symmetrically while in the file. Do not let the fix depend only on the screen: a later refactor of the composable would silently remove it.

Verify: a replacement is written only after the current PIN has been checked in a new app/src/test/kotlin/dev/privacyllc/period/feature/lock/LockSettingsViewModelTest.kt — red before the fix; and bash scripts/prove-guard.sh mutating the write-site guard to if (false) reddens exactly that test.

Found by tracing the app-lock flow at 93ec5b7. **What is true now.** `app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockSettingsScreen.kt:84-93`, the `CONFIRM_TO_CHANGE` branch, calls `viewModel.verifyCurrent(pin) { }` — asynchronous, with an EMPTY callback — and then assigns `mode = Mode.SET_REPLACEMENT` unconditionally, outside the result. `verifyCurrent` (`LockSettingsViewModel.kt:99-113`) only sets a WRONG_PIN message; nothing gates the transition. `SET_REPLACEMENT` calls `setPin`, and `AppLockRepository.setPin` (`core/security/.../AppLockRepository.kt:92-100`) enrolls without verifying anything. So: Change PIN, type any four digits, and you reach 'Choose a PIN' and overwrite the PIN. **What it costs.** Anyone holding the phone while it is unlocked can change the app's PIN. Under this project's no-recovery policy the owner's only way back into her own history is to erase all of it. That is the exact bypass the file's own KDoc says must not be possible (`LockSettingsScreen.kt:45-53`, `LockSettingsViewModel.kt:31-40`). **What to do.** Move the transition inside the verified callback, exactly as `CONFIRM_TO_REMOVE` (`:76-82`) already does — `onAuthoriseChange(pin) { mode = SET_REPLACEMENT }`. Add a guard at the write site too: the ViewModel refuses `setPin` when a PIN already exists and no successful check authorised a replacement, because the place that writes the PIN is the place that must refuse. **Traps.** `CONFIRM_TO_REMOVE` has the same advance-before-async shape and is benign for security, but its wrong-PIN message lands on the Overview instead of the PIN screen (`ConfirmPin.wrong` is dead) — fix it symmetrically while in the file. Do not let the fix depend only on the screen: a later refactor of the composable would silently remove it. Verify: `a replacement is written only after the current PIN has been checked` in a new `app/src/test/kotlin/dev/privacyllc/period/feature/lock/LockSettingsViewModelTest.kt` — red before the fix; and `bash scripts/prove-guard.sh` mutating the write-site guard to `if (false)` reddens exactly that test.