WordPress/two-factor

Fatal Error: PHP 7.2 Syntax Error (breaking whole site)

KZeni opened this issue · 1 comments

KZeni commented

Describe the bug

This plugin says it supports PHP 5.6+, but version 0.8.0 of this plugin released a few hours ago has sites running PHP 7.2.x encounter the following fatal server error site-wide:

Parse error:
syntax error, unexpected ')'
in /wp-content/plugins/two-factor/class-two-factor-core.php
on line 1327

This is a case where PHP 7.2 doesn’t like an extra comma at the end of a list a function parameters. It’s okay to have that trailing comma in an array & things, but it causes site-wide fatal error when doing a similar thing when calling a function on PHP 7.2 (even though that comma isn’t doing anything in the first place so it’s useless while then being a potentially serious issue for some sites.)

As such, 0.8.0 should see a hotfix to address this. Simply changing class-two-factor-core.php around line 1327 from:

$user_message = sprintf(
	'Hello %1$s, an unusually high number of failed login attempts have been detected on your account at %2$s.
	These attempts successfully entered your password, and were only blocked because they failed to enter your second authentication factor. Despite not being able to access your account, this behavior indicates that the attackers have compromised your password. The most common reasons for this are that your password was easy to guess, or was reused on another site which has been compromised.
	To protect your account, your password has been reset, and you will need to create a new one. For advice on setting a strong password, please read %3$s
	To pick a new password, please visit %4$s
	This is an automated notification. If you would like to speak to a site administrator, please contact them directly.',
	esc_html( $user->user_login ),
	home_url(),
	'https://wordpress.org/documentation/article/password-best-practices/',
	esc_url( add_query_arg( 'action', 'lostpassword', wp_login_url() ) ),
);

to instead be:

$user_message = sprintf(
	'Hello %1$s, an unusually high number of failed login attempts have been detected on your account at %2$s.
	These attempts successfully entered your password, and were only blocked because they failed to enter your second authentication factor. Despite not being able to access your account, this behavior indicates that the attackers have compromised your password. The most common reasons for this are that your password was easy to guess, or was reused on another site which has been compromised.
	To protect your account, your password has been reset, and you will need to create a new one. For advice on setting a strong password, please read %3$s
	To pick a new password, please visit %4$s
	This is an automated notification. If you would like to speak to a site administrator, please contact them directly.',
	esc_html( $user->user_login ),
	home_url(),
	'https://wordpress.org/documentation/article/password-best-practices/',
	esc_url( add_query_arg( 'action', 'lostpassword', wp_login_url() ) )
);

fixes the issue (notice how the very last comma that’s not separating anything has been removed) with it appearing this might be the only place this problematic comma is located.

I've also posted this on https://wordpress.org/support/topic/fatal-error-php-7-2-syntax-error-breaking-whole-site-3/ and have a pull request at #547

Steps to Reproduce

  1. Install & activate version 0.8.0 of this plugin on a WordPress site running on PHP 7.2.
  2. Encounter a site-wide 500 server error as detailed above.

Screenshots, screen recording, code snippet

No response

Environment information

  • PHP 7.2.x
  • The rest of the details don't matter. It's a PHP version compatibility bug.

Please confirm that you have searched existing issues in this repository.

Yes

Please confirm that you have tested with all plugins deactivated except Two-Factor.

Yes

Fixed by #547