WordPress / WordPress/two-factor

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

Open Beginner friendly
#956 0 comments 0 reactions 0 assignees View on GitHub

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 => appendrest_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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.