diff --git a/class-two-factor-core.php b/class-two-factor-core.php index c034c53d..e6924c5d 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -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; @@ -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; @@ -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 ) ) { /** @@ -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. self::login_html( $user, $login_nonce['key'], $redirect_to ); } @@ -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; + } + + // 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, '@' ) ) { @@ -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. + $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. + $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 ) { @@ -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 ( ! 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; } @@ -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' ); } @@ -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. + $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; @@ -2384,11 +2396,7 @@ public static function get_current_user_session() { * @return boolean */ public static function rememberme() { - $rememberme = false; - - if ( ! empty( $_REQUEST['rememberme'] ) ) { - $rememberme = true; - } + $rememberme = ! empty( $_REQUEST['rememberme'] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Non-destructive display/flow flag; value normalized to bool below. /** * Filters whether the login session should persist between browser sessions. diff --git a/providers/class-two-factor-provider.php b/providers/class-two-factor-provider.php index 97fda12a..cdc70d15 100644 --- a/providers/class-two-factor-provider.php +++ b/providers/class-two-factor-provider.php @@ -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. diff --git a/tests/class-two-factor-core.php b/tests/class-two-factor-core.php index 700a7ac9..3ece1cc0 100644 --- a/tests/class-two-factor-core.php +++ b/tests/class-two-factor-core.php @@ -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. *