WordPress / WordPress/two-factor
generate_codes() with 'method' => 'append' stores an empty code when the user has none
Nobody has claimed this yet.
- Dominant language
- PHP
- Stars
- 825
- Forks
- 187
- Avg merge
- 2d 11h
- Merged PRs (30d)
- 17
Description
Generating backup codes with 'method' => 'append' for a user who has none stores an extra, empty code. The empty entry can never be validated and is never removed, so codes_remaining_for_user() permanently reports one more code than the user actually has.
Line references are against master at 6245be6. Reproduced on WordPress 7.0.3 with Two Factor 0.16.0 installed from wordpress.org.
Reproduction
$user = get_user_by( 'login', 'someone' ); // no backup codes yet
$codes = Two_Factor_Backup_Codes::get_instance()->generate_codes( $user, array( 'method' => 'append' ) );
count( $codes ); // 10 — correct
count( get_user_meta( $user->ID, '_two_factor_backup_codes', true ) ); // 11
Two_Factor_Backup_Codes::codes_remaining_for_user( $user ); // 11
Index 0 of the stored array is an empty string; the ten real hashes follow it.
Root cause
providers/class-two-factor-backup-codes.php:319-321
// Append or replace (default).
if ( isset( $args['method'] ) && 'append' === $args['method'] ) {
$codes_hashed = (array) get_user_meta( $user->ID, self::BACKUP_CODES_META_KEY, true );
}
get_user_meta( ..., true ) returns '' when the key is absent, and (array) '' evaluates to array( '' ) rather than array(). The ten generated hashes are then appended to an array that already contains one empty string.
Worth noting the rest of the class already guards this correctly — codes_remaining_for_user() at :400-403 and validate_code() at :491-493 both check is_array() before use. Line 320 is the only place that relies on a bare cast.
Why it matters
The empty entry is permanent. validate_code() at :490-501 iterates the stored hashes and only calls delete_code() on a match; wp_check_password( $code, '' ) never matches, so nothing ever removes it.
That leads to two user-facing consequences once the real codes are spent:
A user with zero usable codes is still offered backup codes at login. is_available_for_user() at :214-220 returns true whenever codes_remaining_for_user() is above zero. With only the empty entry left, the count is 1, so Two_Factor_Backup_Codes stays in get_available_providers_for_user() and is presented as a recovery option on the two-factor prompt. Someone locked out of their primary factor selects it, and no code they hold can ever work.
The low-codes warning states the wrong number. The notice at :157-170 reports $count, so a user with one real code left is told they have two, and a user with none is told they have one — advising them they still have a way in when they do not.
Suggested fix
if ( isset( $args['method'] ) && 'append' === $args['method'] ) {
$existing = get_user_meta( $user->ID, self::BACKUP_CODES_META_KEY, true );
$codes_hashed = is_array( $existing ) ? $existing : array();
}
This matches the is_array() guard the class already uses elsewhere.
A migration for already-affected users may be worth considering separately — filtering empty values out of codes_remaining_for_user() would correct the count for existing installs, though it would not remove the stored entry.
Scope
No caller inside the plugin passes method => append — rest_generate_codes() at :345-356 builds $args from the request and the REST route does not expose it. So this only affects code calling generate_codes() directly, which is how I hit it: an add-on generating a first set of codes for a user during an enrollment flow, passing append defensively so that an existing set could never be clobbered. On a user with no codes, that defensive flag is what produces the extra entry.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in providers/class-two-factor-backup-codes.php at generate_codes(), especially lines 319-321, and review the existing is_array() guards in codes_remaining_for_user() and validate_code(). Reproduce the append case with a user who has no stored codes; done means only the generated codes are stored and the remaining-code count matches them.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php, wordpress
- Domain
- authentication, security
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 84/100