Skip to content

fix(security): drop the biometric blob when the OS reports no enrolled biometric [PERA-4702] - #1092

Draft
yasin-ce wants to merge 3 commits into
mainfrom
yasince/fix/PERA-4702
Draft

fix(security): drop the biometric blob when the OS reports no enrolled biometric [PERA-4702]#1092
yasin-ce wants to merge 3 commits into
mainfrom
yasince/fix/PERA-4702

Conversation

@yasin-ce

Copy link
Copy Markdown
Collaborator

Description

  • checkBiometricsEnabled now reconciles instead of only reading: when the stored biometric blob exists but the OS reports no enrolled biometric, the blob is deleted and the answer is false.
  • refreshBiometricsBinding routes through that same check rather than a bare hasSecret, so a PIN change cannot rewrite a blob whose OS enrollment is already gone.

Related Issues

Checklist

  • Have you tested your changes locally?
  • Have you reviewed the code for any potential issues?
  • Have you documented any necessary changes in the project's documentation?
  • Have you added any necessary tests for your changes?
  • Have you updated any relevant dependencies?

Additional Notes

The threat this closes: the blob is the app's only record that biometric unlock was opted into, so one that outlives its enrollment silently re-arms unlock the moment the user enrols a new biometric — a fingerprint added after the fact could open the wallet without the in-app toggle ever being touched. The PIN record is a separate secret, so dropping the blob never costs access.

The second commit documents the side effect on the hook's return type rather than renaming the method: checkBiometricsEnabled has ~8 call sites and a dozen test doubles, and a rename would have buried a 60-line security fix in mechanical churn. Happy to rename in a follow-up if you would rather the name carried it.

One thing worth a reviewer's eye: if removeSecret throws, this now rejects rather than returning false, so a caller expecting a boolean on the fail-closed path gets a rejection instead. I have not audited every call site for that.

@yasin-ce
yasin-ce force-pushed the yasince/fix/PERA-4702 branch from 47a6d7e to 7d2a7ad Compare July 30, 2026 13:21
@filipetamota

Copy link
Copy Markdown
Collaborator

@yasin-ce

QA'd this on Android — blob-clearing works, but two gaps before this closes PERA-4702:

  1. Toggle stays ON after revocation. Remove all fingerprints (app running) → lock → PIN fallback works, but Settings still shows biometrics enabled. isEnabled is per-instance useState, so the reconcile only updates the lock screen's copy, not Settings'. Needs a shared source of truth (store) so all consumers see it cleared.

  2. Killed-app re-enroll still silently unlocks — the ticket's actual vuln. Kill app → remove all fingerprints → re-add one → cold launch unlocks with no in-app opt-in. The JS reconcile can't catch this (on launch the OS already reports a valid enrollment, and the blob isn't biometric-bound). Real fix is the keystore binding (setInvalidatedByBiometricEnrollment, PERA-4424).

Suggest either folding in the keystore binding, or rescoping this to "foreground reconcile + toggle sync" and keeping the revocation vuln open under PERA-4424 — right now the wording reads as if revocation is fully handled.

@yasin-ce
yasin-ce force-pushed the yasince/fix/PERA-4702 branch from 7d2a7ad to 4980d8d Compare July 31, 2026 09:22
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