Skip to content

Validate remote-verify QR before persisting account state#331

Merged
thestinger merged 1 commit intoGrapheneOS:mainfrom
thomasbuilds:patch-1
Apr 27, 2026
Merged

Validate remote-verify QR before persisting account state#331
thestinger merged 1 commit intoGrapheneOS:mainfrom
thomasbuilds:patch-1

Conversation

@thomasbuilds
Copy link
Copy Markdown
Contributor

Hardens the Stage.EnableRemoteVerify branch of the QR scan result handler in AttestationActivity so that a malformed account QR can no longer crash the app or leave remote verification in a half-enabled state.

Problem

In AttestationActivity.java the handler did:

PreferenceManager.getDefaultSharedPreferences(this).edit()
        .putLong(RemoteVerifyJob.KEY_USER_ID, Long.parseLong(values[1]))
        .putString(RemoteVerifyJob.KEY_SUBSCRIBE_KEY, values[2])
        .apply();
try {
    RemoteVerifyJob.schedule(this, Integer.parseInt(values[3]));
    snackbar.setText(R.string.enable_remote_verify_success).show();
} catch (final NumberFormatException e) {
    snackbar.setText(R.string.scanned_invalid_account_qr_code).show();
}

Two issues:

  1. Long.parseLong(values[1]) is outside the try. Any QR of the form attestation.app <non-numeric> … triggers an uncaught NumberFormatException in the ActivityResultLauncher lambda and crashes the activity. Only the interval field was guarded.
  2. The user id and subscribe key are written to SharedPreferences before the interval is parsed and the job is scheduled. If the interval is malformed, the prefs are already persisted, so:
    • RemoteVerifyJob.isEnabled() returns true, hiding the "Enable remote verification" button in onResume().
    • RemoteVerifyJob.isScheduled() returns false — no job runs.
    • The user has to use the "Disable remote verification" menu item to recover.

Fix

Parse both userId and interval inside a single try / catch (NumberFormatException). Only after both parse successfully are preferences committed and the job scheduled. Behavior on a valid QR is unchanged.

The EnableRemoteVerify QR handler called Long.parseLong(values[1]) outside the try/catch that guarded Integer.parseInt(values[3]), so a QR with a non-numeric user id crashed the activity instead of showing the "invalid account QR code" snackbar.

It also wrote KEY_USER_ID and KEY_SUBSCRIBE_KEY to SharedPreferences *before* scheduling the job. If parsing the interval threw, the prefs were already committed: RemoteVerifyJob.isEnabled() then returned true (hiding the "Enable remote verification" button) while no job was ever scheduled, leaving the user with no in-app way to recover short of "Disable remote verification".

Parse both numeric fields inside a single try/catch and only commit preferences and schedule the job once every field has been validated.
@RankoR RankoR self-requested a review April 27, 2026 17:38
@thestinger thestinger merged commit 55d5d82 into GrapheneOS:main Apr 27, 2026
2 checks passed
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.

4 participants