Skip to content

#831 Fix nonce verification in 2FA flow #833

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
PANawkar wants to merge 3 commits into
base: master
Choose a base branch
from
+68 −24
Open

#831 Fix nonce verification in 2FA flow #833

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
09bb677
Fix nonce verification in 2FA flow
Mar 12, 2026
498eb6d
Harden nonce/request handling, fix phpcs comments, add reset notice n…
Mar 12, 2026
6a1ac8e
use !empty() for rememberme check and simplify rememberme()
Mar 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 30 additions & 18 deletions class-two-factor-core.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,12 @@ public static function get_user_two_factor_revalidate_url( $interim = false ) {
* @return boolean
*/
public static function is_valid_user_action( $user_id, $action ) {
$request_nonce = isset( $_REQUEST[ self::USER_SETTINGS_ACTION_NONCE_QUERY_ARG ] ) ? wp_unslash( $_REQUEST[ self::USER_SETTINGS_ACTION_NONCE_QUERY_ARG ] ) : '';
$request_nonce_raw = isset( $_REQUEST[ self::USER_SETTINGS_ACTION_NONCE_QUERY_ARG ] ) ? wp_unslash( $_REQUEST[ self::USER_SETTINGS_ACTION_NONCE_QUERY_ARG ] ) : ''; // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- Value sanitized and then only passed to wp_verify_nonce().
if ( ! is_scalar( $request_nonce_raw ) ) {
$request_nonce = '';
} else {
$request_nonce = sanitize_text_field( (string) $request_nonce_raw );
}

if ( ! $user_id || ! $action || ! $request_nonce ) {
return false;
Expand All @@ -473,8 +478,8 @@ public static function is_valid_user_action( $user_id, $action ) {
*/
public static function current_user_being_edited() {
// Try to resolve the user ID from the request first.
if ( ! empty( $_REQUEST['user_id'] ) ) {
$user_id = intval( $_REQUEST['user_id'] );
if ( ! empty( $_REQUEST['user_id'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Nonce verified in trigger_user_settings_action() via is_valid_user_action() before any state change.
$user_id = intval( $_REQUEST['user_id'] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Nonce verified in trigger_user_settings_action() via is_valid_user_action() before any state change.

if ( current_user_can( 'edit_user', $user_id ) ) {
return $user_id;
Expand All @@ -493,8 +498,9 @@ public static function current_user_being_edited() {
* @return void
*/
public static function trigger_user_settings_action() {
$action = isset( $_REQUEST[ self::USER_SETTINGS_ACTION_QUERY_VAR ] ) ? wp_unslash( $_REQUEST[ self::USER_SETTINGS_ACTION_QUERY_VAR ] ) : '';
$user_id = self::current_user_being_edited();
$action_raw = isset( $_REQUEST[ self::USER_SETTINGS_ACTION_QUERY_VAR ] ) ? wp_unslash( $_REQUEST[ self::USER_SETTINGS_ACTION_QUERY_VAR ] ) : ''; // phpcs:ignore WordPress.Security.NonceVerification.Recommended, WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- Value sanitized below; nonce verified in is_valid_user_action() before do_action.
$action = ( is_scalar( $action_raw ) && (string) $action_raw !== '' ) ? sanitize_key( (string) $action_raw ) : '';
$user_id = self::current_user_being_edited();

if ( self::is_valid_user_action( $user_id, $action ) ) {
/**
Expand Down Expand Up @@ -906,7 +912,7 @@ public static function show_two_factor_login( $user ) {
wp_die( esc_html__( 'Failed to create a login nonce.', 'two-factor' ) );
}

$redirect_to = isset( $_REQUEST['redirect_to'] ) ? $_REQUEST['redirect_to'] : admin_url();
$redirect_to = isset( $_REQUEST['redirect_to'] ) ? esc_url_raw( wp_unslash( $_REQUEST['redirect_to'] ) ) : admin_url(); // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Value only used for redirect; auth protected by 2FA login nonce later.

Comment thread
PANawkar marked this conversation as resolved.
self::login_html( $user, $login_nonce['key'], $redirect_to );
}
Expand Down Expand Up @@ -960,6 +966,12 @@ public static function maybe_show_reset_password_notice( $errors ) {
return $errors;
}

// Verify login form nonce when present (e.g. wp-login.php); skip only when nonce is not sent (custom login forms).
if ( isset( $_POST['_wpnonce'] ) && ! wp_verify_nonce( sanitize_text_field( wp_unslash( $_POST['_wpnonce'] ) ), 'log-in' ) ) {
return $errors;
}
Comment thread
PANawkar marked this conversation as resolved.

// phpcs:ignore WordPress.Security.NonceVerification.Missing -- Nonce verified above when _wpnonce present; absent for custom login forms.
$user_name = sanitize_user( wp_unslash( $_POST['log'] ) );
$attempted_user = get_user_by( 'login', $user_name );
if ( ! $attempted_user && str_contains( $user_name, '@' ) ) {
Expand Down Expand Up @@ -1487,11 +1499,11 @@ public static function rest_api_can_edit_user_and_update_two_factor_options( $us
* @since 0.2.0
*/
public static function login_form_validate_2fa() {
$wp_auth_id = ! empty( $_REQUEST['wp-auth-id'] ) ? absint( $_REQUEST['wp-auth-id'] ) : 0;
$nonce = ! empty( $_REQUEST['wp-auth-nonce'] ) ? wp_unslash( $_REQUEST['wp-auth-nonce'] ) : '';
$provider = ! empty( $_REQUEST['provider'] ) ? wp_unslash( $_REQUEST['provider'] ) : '';
$redirect_to = ! empty( $_REQUEST['redirect_to'] ) ? wp_unslash( $_REQUEST['redirect_to'] ) : '';
$is_post_request = ( 'POST' === strtoupper( $_SERVER['REQUEST_METHOD'] ) );
$wp_auth_id = ! empty( $_REQUEST['wp-auth-id'] ) ? absint( $_REQUEST['wp-auth-id'] ) : 0; // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Nonce verified in _login_form_validate_2fa() via verify_login_nonce() before any use.
Comment thread
PANawkar marked this conversation as resolved.
$nonce = ( isset( $_REQUEST['wp-auth-nonce'] ) && is_scalar( $_REQUEST['wp-auth-nonce'] ) ) ? sanitize_text_field( wp_unslash( (string) $_REQUEST['wp-auth-nonce'] ) ) : ''; // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Nonce verified in _login_form_validate_2fa() before any use.
$provider = ! empty( $_REQUEST['provider'] ) ? sanitize_text_field( wp_unslash( $_REQUEST['provider'] ) ) : ''; // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Nonce verified in _login_form_validate_2fa() before any use.
$redirect_to = ! empty( $_REQUEST['redirect_to'] ) ? esc_url_raw( wp_unslash( $_REQUEST['redirect_to'] ) ) : ''; // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Nonce verified in _login_form_validate_2fa() before any use.
Comment thread
PANawkar marked this conversation as resolved.
$is_post_request = isset( $_SERVER['REQUEST_METHOD'] ) && 'POST' === strtoupper( $_SERVER['REQUEST_METHOD'] ); // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.MissingUnslash, WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- REQUEST_METHOD is not user input.
$user = get_user_by( 'id', $wp_auth_id );

if ( ! $wp_auth_id || ! $nonce || ! $user ) {
Expand Down Expand Up @@ -1553,7 +1565,7 @@ public static function _login_form_validate_2fa( $user, $nonce = '', $provider =
delete_user_meta( $user->ID, self::USER_FAILED_LOGIN_ATTEMPTS_KEY );

$rememberme = false;
if ( isset( $_REQUEST['rememberme'] ) && $_REQUEST['rememberme'] ) {
if ( isset( $_REQUEST['rememberme'] ) && $_REQUEST['rememberme'] ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended, WordPress.Security.ValidatedSanitizedInput.MissingUnslash, WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- Request read only after successful verify_login_nonce() in this request.
Copy link
Copy Markdown
Contributor

@sjinks sjinks Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if ( isset( $_REQUEST['rememberme'] ) && $_REQUEST['rememberme'] ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended, WordPress.Security.ValidatedSanitizedInput.MissingUnslash, WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- Request read only after successful verify_login_nonce() in this request.
if ( ! empty( $_REQUEST['rememberme'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended, WordPress.Security.ValidatedSanitizedInput.MissingUnslash, WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- Request read only after successful verify_login_nonce() in this request.

$rememberme = true;
}

Expand Down Expand Up @@ -1594,7 +1606,7 @@ public static function _login_form_validate_2fa( $user, $nonce = '', $provider =
$interim_login = isset( $_REQUEST['interim-login'] ); // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited,WordPress.Security.NonceVerification.Recommended

if ( $interim_login ) {
$customize_login = isset( $_REQUEST['customize-login'] );
$customize_login = isset( $_REQUEST['customize-login'] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended, WordPress.Security.ValidatedSanitizedInput.MissingUnslash, WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- Request read only after successful verify_login_nonce() in this request.
if ( $customize_login ) {
wp_enqueue_script( 'customize-base' );
}
Expand Down Expand Up @@ -1627,10 +1639,10 @@ public static function _login_form_validate_2fa( $user, $nonce = '', $provider =
* @since 0.9.0
*/
public static function login_form_revalidate_2fa() {
$nonce = ! empty( $_REQUEST['wp-auth-nonce'] ) ? wp_unslash( $_REQUEST['wp-auth-nonce'] ) : '';
$provider = ! empty( $_REQUEST['provider'] ) ? sanitize_text_field( wp_unslash( $_REQUEST['provider'] ) ) : false;
$redirect_to = ! empty( $_REQUEST['redirect_to'] ) ? wp_unslash( $_REQUEST['redirect_to'] ) : admin_url();
$is_post_request = ( 'POST' === strtoupper( $_SERVER['REQUEST_METHOD'] ) );
$nonce = ( isset( $_REQUEST['wp-auth-nonce'] ) && is_scalar( $_REQUEST['wp-auth-nonce'] ) ) ? sanitize_text_field( wp_unslash( (string) $_REQUEST['wp-auth-nonce'] ) ) : ''; // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Nonce verified in _login_form_revalidate_2fa() for POST before processing.
$provider = ! empty( $_REQUEST['provider'] ) ? sanitize_text_field( wp_unslash( $_REQUEST['provider'] ) ) : false; // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Nonce verified in _login_form_revalidate_2fa() for POST before processing.
$redirect_to = ! empty( $_REQUEST['redirect_to'] ) ? esc_url_raw( wp_unslash( $_REQUEST['redirect_to'] ) ) : admin_url(); // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Nonce verified in _login_form_revalidate_2fa() for POST before processing.
Comment thread
PANawkar marked this conversation as resolved.
$is_post_request = isset( $_SERVER['REQUEST_METHOD'] ) && 'POST' === strtoupper( $_SERVER['REQUEST_METHOD'] ); // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.MissingUnslash, WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- REQUEST_METHOD is not user input.

self::_login_form_revalidate_2fa( $nonce, $provider, $redirect_to, $is_post_request );
exit;
Expand Down Expand Up @@ -2386,7 +2398,7 @@ public static function get_current_user_session() {
public static function rememberme() {
$rememberme = false;

if ( ! empty( $_REQUEST['rememberme'] ) ) {
if ( ! empty( $_REQUEST['rememberme'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Non-destructive display/flow flag; value normalized to bool below.
$rememberme = true;
}
Copy link
Copy Markdown
Contributor

@sjinks sjinks Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe

$rememberme = ! empty( $_REQUEST['rememberme'] );

?

Copy link
Copy Markdown
Author

@PANawkar PANawkar Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, we can use that


Expand Down
4 changes: 2 additions & 2 deletions providers/class-two-factor-provider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,11 @@ public static function get_code( $length = 8, $chars = '1234567890' ) {
* @return false|string Auth code on success, false if the field is not set or not expected length.
*/
public static function sanitize_code_from_request( $field, $length = 0 ) {
if ( empty( $_REQUEST[ $field ] ) ) {
if ( empty( $_REQUEST[ $field ] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Caller (core) verifies nonce before provider processing.
return false;
}

$code = wp_unslash( $_REQUEST[ $field ] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended, handled by the core method already.
$code = wp_unslash( $_REQUEST[ $field ] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended, WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- Caller (core) verifies nonce; value normalized below.
$code = preg_replace( '/\s+/', '', $code );

// Maybe validate the length.
Expand Down
36 changes: 36 additions & 0 deletions tests/class-two-factor-core.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -785,6 +785,42 @@ public function test_reset_notice_when_password_was_reset() {
$this->assertSame( 'two_factor_password_reset', $errors->get_error_code() );
}

/**
* Test no reset notice when _wpnonce is present but invalid.
*
* @covers Two_Factor_Core::maybe_show_reset_password_notice()
*/
public function test_no_reset_notice_when_wpnonce_invalid() {
$user = self::factory()->user->create_and_get();
$errors = new WP_Error( 'incorrect_password', 'Incorrect password' );
$_POST['log'] = $user->user_login;
$_POST['_wpnonce'] = 'invalid-nonce';

update_user_meta( $user->ID, Two_Factor_Core::USER_PASSWORD_WAS_RESET_KEY, true );
Two_Factor_Core::maybe_show_reset_password_notice( $errors );
$this->assertCount( 1, $errors->get_error_codes() );
$this->assertSame( 'incorrect_password', $errors->get_error_code() );
unset( $_POST['_wpnonce'] );
}

/**
* Test reset notice when _wpnonce is present and valid for log-in.
*
* @covers Two_Factor_Core::maybe_show_reset_password_notice()
*/
public function test_reset_notice_when_wpnonce_valid() {
$user = self::factory()->user->create_and_get();
$errors = new WP_Error( 'incorrect_password', 'Incorrect password' );
$_POST['log'] = $user->user_login;
$_POST['_wpnonce'] = wp_create_nonce( 'log-in' );

update_user_meta( $user->ID, Two_Factor_Core::USER_PASSWORD_WAS_RESET_KEY, true );
Two_Factor_Core::maybe_show_reset_password_notice( $errors );
$this->assertCount( 1, $errors->get_error_codes() );
$this->assertSame( 'two_factor_password_reset', $errors->get_error_code() );
unset( $_POST['_wpnonce'] );
}

/**
* Test clear password reset notice.
*
Expand Down
Loading