generate_codes() with 'method' => 'append' stores an empty code when the user has none

[dknauss]

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.