From e6cdf02c62520784623cce196ea3da73e7596766 Mon Sep 17 00:00:00 2001 From: Rebecca Hum Date: Tue, 24 Oct 2023 11:54:05 -0600 Subject: [PATCH 1/4] Security: Use ambiguous error message in forgot password for multisites --- security/login-error.php | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/security/login-error.php b/security/login-error.php index 517c08daab..8e3fdac226 100644 --- a/security/login-error.php +++ b/security/login-error.php @@ -1,5 +1,8 @@ get_error_codes(); $err_types = [ @@ -35,5 +43,25 @@ function use_ambiguous_login_error( $error ): string { return (string) $error; } - add_filter( 'login_errors', __NAMESPACE__ . '\use_ambiguous_login_error', 99, 1 ); + +/** + * Use a message that does not reveal the type of login error in an attempted brute-force on forget password. + * + * @param WP_Error $errors WP Error object. + * + * @return WP_Error $errors WP Error object. + * + * @since 1.1 + */ +function use_ambiguous_confirmation( $errors ): WP_Error { + if ( isset( $_GET['checkemail'] ) && 'confirm' === $_GET['checkemail'] ) { + foreach ( $errors as &$err ) { + if ( isset( $err['confirm'][0] ) ) { + $err['confirm'][0] = FORGET_PWD_MESSAGE; + } + } + } + return $errors; +} +add_filter( 'wp_login_errors', __NAMESPACE__ . '\use_ambiguous_confirmation', 99 ); From 2b847474d92956e4df8313c0979c6a77d4a1f0db Mon Sep 17 00:00:00 2001 From: Rebecca Hum Date: Tue, 24 Oct 2023 12:33:39 -0600 Subject: [PATCH 2/4] Fix linting --- security/login-error.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/security/login-error.php b/security/login-error.php index 8e3fdac226..6cb8c5f395 100644 --- a/security/login-error.php +++ b/security/login-error.php @@ -1,5 +1,6 @@ Date: Tue, 24 Oct 2023 23:15:00 +0300 Subject: [PATCH 3/4] Add tests --- security/login-error.php | 12 +++---- tests/security/test-login-error.php | 53 +++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 6 deletions(-) create mode 100644 tests/security/test-login-error.php diff --git a/security/login-error.php b/security/login-error.php index 6cb8c5f395..e18bf88f85 100644 --- a/security/login-error.php +++ b/security/login-error.php @@ -7,11 +7,11 @@ /** * Use a login message that does not reveal the type of login error in an attempted brute-force. - * + * * @param string $error Login error message. - * + * * @return string $error Login error message. - * + * * @since 1.1 */ function use_ambiguous_login_error( $error ): string { @@ -48,11 +48,11 @@ function use_ambiguous_login_error( $error ): string { /** * Use a message that does not reveal the type of login error in an attempted brute-force on forget password. - * + * * @param WP_Error $errors WP Error object. - * + * * @return WP_Error $errors WP Error object. - * + * * @since 1.1 */ function use_ambiguous_confirmation( $errors ): WP_Error { diff --git a/tests/security/test-login-error.php b/tests/security/test-login-error.php new file mode 100644 index 0000000000..d3d4506bbe --- /dev/null +++ b/tests/security/test-login-error.php @@ -0,0 +1,53 @@ +add( + 'confirm', + sprintf( + 'Check your email for the confirmation link, then visit the login page.', + wp_login_url() + ), + 'message' + ); + + $_GET['checkemail'] = 'confirm'; + $actual = apply_filters( 'wp_login_errors', $errors, admin_url() ); + + self::assertInstanceOf( WP_Error::class, $actual ); + self::assertContains( FORGET_PWD_MESSAGE, $actual->get_error_messages( 'confirm' ) ); + } + + public function test_ambiguous_reset(): void { + global $errors; + + $message = 'Something went terribly wrong'; + + // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited + $errors = new WP_Error(); + $errors->add( 'error', $message ); + + $_GET['action'] = 'lostpassword'; + + $actual = apply_filters( 'login_errors', $message ); + self::assertSame( FORGET_PWD_MESSAGE, $actual ); + } +} From 998853903822be3f45c468f1d88dea6493e5bc8d Mon Sep 17 00:00:00 2001 From: Rebecca Hum Date: Tue, 24 Oct 2023 14:57:13 -0600 Subject: [PATCH 4/4] use error api instead of traversing mebbe fix test with type checking --- security/login-error.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/security/login-error.php b/security/login-error.php index e18bf88f85..21f0ebaf18 100644 --- a/security/login-error.php +++ b/security/login-error.php @@ -57,12 +57,13 @@ function use_ambiguous_login_error( $error ): string { */ function use_ambiguous_confirmation( $errors ): WP_Error { if ( isset( $_GET['checkemail'] ) && 'confirm' === $_GET['checkemail'] ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended - foreach ( $errors as &$err ) { - if ( isset( $err['confirm'][0] ) ) { - $err['confirm'][0] = FORGET_PWD_MESSAGE; - } + $messages = $errors->get_error_messages( 'confirm' ); + if ( ! empty( $messages ) ) { + $errors->remove( 'confirm' ); + $errors->add( 'confirm', FORGET_PWD_MESSAGE, 'message' ); } } + return $errors; } add_filter( 'wp_login_errors', __NAMESPACE__ . '\use_ambiguous_confirmation', 99 );