Skip to content

Commit 77084c1

Browse files
committed
Fix PHPCS and PHPStan issues across multiple files
- Add PHPStan bootstrap file for runtime constants (TWO_FACTOR_DIR, TWO_FACTOR_VERSION) - Add missing properties ($new, $last_used) to Registration class - Fix PHPDoc types for show_two_factor_login, process_provider, authentication_page, rename_link, delete_link, and pack64 - Fix undefined variable bug in wp_ajax_inline_save - Add input validation, sanitization, and wp_unslash for $_POST/$_REQUEST usage - Remove redundant isset($user->ID) checks and always-true conditions - Cast base_convert() result to int for array offset usage
1 parent df7541f commit 77084c1

7 files changed

Lines changed: 53 additions & 25 deletions

File tree

class-two-factor-core.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -897,7 +897,7 @@ public static function is_api_request() {
897897
*
898898
* @since 0.2.0
899899
*
900-
* @param WP_User $user WP_User object of the logged-in user.
900+
* @param WP_User|false $user WP_User object of the logged-in user.
901901
*/
902902
public static function show_two_factor_login( $user ) {
903903
if ( ! $user ) {
@@ -1708,9 +1708,9 @@ public static function _login_form_revalidate_2fa( $nonce = '', $provider = '',
17081708
*
17091709
* @since 0.9.0
17101710
*
1711-
* @param object $provider The Two Factor Provider.
1712-
* @param WP_User $user The user being authenticated.
1713-
* @param bool $is_post_request Whether the request is a POST request.
1711+
* @param object|null $provider The Two Factor Provider.
1712+
* @param WP_User $user The user being authenticated.
1713+
* @param bool $is_post_request Whether the request is a POST request.
17141714
* @return false|WP_Error|true WP_Error when an error occurs, true when the user is authenticated, false if no action occurred.
17151715
*/
17161716
public static function process_provider( $provider, $user, $is_post_request ) {
@@ -2013,7 +2013,7 @@ public static function user_two_factor_options( $user ) {
20132013
<h2><?php esc_html_e( 'Two-Factor Options', 'two-factor' ); ?></h2>
20142014

20152015
<?php foreach ( $notices as $notice_type => $notice ) : ?>
2016-
<div class="<?php echo esc_attr( $notice_type ? 'notice inline notice-' . $notice_type : '' ); ?>">
2016+
<div class="<?php echo esc_attr( 'notice inline notice-' . $notice_type ); ?>">
20172017
<p><?php echo wp_kses_post( $notice ); ?></p>
20182018
</div>
20192019
<?php endforeach; ?>

includes/Yubico/U2F.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,12 @@ class Registration
486486

487487
/** The counter associated with this registration */
488488
public $counter = -1;
489+
490+
/** Whether this is a new registration */
491+
public $new;
492+
493+
/** Timestamp when this registration was last used */
494+
public $last_used;
489495
}
490496

491497
/**

phpstan-bootstrap.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
/**
3+
* PHPStan bootstrap file.
4+
*
5+
* Defines constants that are set at runtime in two-factor.php
6+
* but unreachable during static analysis because of the ABSPATH guard.
7+
*/
8+
9+
define( 'TWO_FACTOR_DIR', __DIR__ . '/' );
10+
define( 'TWO_FACTOR_VERSION', '0.15.0' );

phpstan.dist.neon

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ includes:
22
- vendor/szepeviktor/phpstan-wordpress/extension.neon
33
parameters:
44
level: 0
5+
bootstrapFiles:
6+
- phpstan-bootstrap.php
57
paths:
68
- includes
79
- providers

providers/class-two-factor-email.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ public function generate_and_email_token( $user ) {
328328
*
329329
* @since 0.1-dev
330330
*
331-
* @param WP_User $user WP_User object of the logged-in user.
331+
* @param WP_User|false $user WP_User object of the logged-in user.
332332
*/
333333
public function authentication_page( $user ) {
334334
if ( ! $user ) {
@@ -379,7 +379,7 @@ public function authentication_page( $user ) {
379379
* @return boolean
380380
*/
381381
public function pre_process_authentication( $user ) {
382-
if ( isset( $user->ID ) && isset( $_REQUEST[ self::INPUT_NAME_RESEND_CODE ] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- non-distructive option that relies on user state.
382+
if ( isset( $_REQUEST[ self::INPUT_NAME_RESEND_CODE ] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- non-distructive option that relies on user state.
383383
$this->generate_and_email_token( $user );
384384
return true;
385385
}
@@ -397,7 +397,7 @@ public function pre_process_authentication( $user ) {
397397
*/
398398
public function validate_authentication( $user ) {
399399
$code = $this->sanitize_code_from_request( 'two-factor-email-code' );
400-
if ( ! isset( $user->ID ) || ! $code ) {
400+
if ( ! $code ) {
401401
return false;
402402
}
403403

providers/class-two-factor-fido-u2f-admin.php

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -237,11 +237,15 @@ public static function show_user_profile( $user ) {
237237
* @return void|never
238238
*/
239239
public static function catch_submission( $user_id ) {
240-
if ( ! empty( $_REQUEST['do_new_security_key'] ) ) {
240+
if ( ! empty( $_REQUEST['do_new_security_key'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Nonce is verified immediately below.
241241
check_admin_referer( "user_security_keys-{$user_id}", '_nonce_user_security_keys' );
242242

243+
if ( ! isset( $_POST['u2f_response'] ) ) {
244+
return;
245+
}
246+
243247
try {
244-
$response = json_decode( stripslashes( $_POST['u2f_response'] ) );
248+
$response = json_decode( wp_unslash( $_POST['u2f_response'] ) ); // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- JSON data decoded immediately.
245249
$reg = Two_Factor_FIDO_U2F::$u2f->doRegister( get_user_meta( $user_id, self::REGISTER_DATA_USER_META_KEY, true ), $response );
246250
$reg->new = true;
247251

@@ -277,8 +281,8 @@ public static function catch_submission( $user_id ) {
277281
public static function catch_delete_security_key() {
278282
$user_id = Two_Factor_Core::current_user_being_edited();
279283

280-
if ( ! empty( $user_id ) && ! empty( $_REQUEST['delete_security_key'] ) ) {
281-
$slug = $_REQUEST['delete_security_key'];
284+
if ( ! empty( $user_id ) && ! empty( $_REQUEST['delete_security_key'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Nonce requires the slug value, verified immediately below.
285+
$slug = sanitize_text_field( wp_unslash( $_REQUEST['delete_security_key'] ) );
282286

283287
check_admin_referer( "delete_security_key-{$slug}", '_nonce_delete_security_key' );
284288

@@ -297,10 +301,10 @@ public static function catch_delete_security_key() {
297301
* @access public
298302
* @static
299303
*
300-
* @param array $item The current item.
304+
* @param object $item The current item.
301305
* @return string
302306
*/
303-
public static function rename_link( $item ) {
307+
public static function rename_link( $item ) { // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter.Found -- Required by WP_List_Table column callback interface.
304308
return sprintf( '<a href="#" class="editinline">%s</a>', esc_html__( 'Rename', 'two-factor' ) );
305309
}
306310

@@ -312,7 +316,7 @@ public static function rename_link( $item ) {
312316
* @access public
313317
* @static
314318
*
315-
* @param array $item The current item.
319+
* @param object $item The current item.
316320
* @return string
317321
*/
318322
public static function delete_link( $item ) {
@@ -345,13 +349,23 @@ public static function wp_ajax_inline_save() {
345349
wp_die();
346350
}
347351

348-
foreach ( $security_keys as &$key ) {
349-
if ( $key->keyHandle === $_POST['keyHandle'] ) { // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase
352+
$key = null;
353+
foreach ( $security_keys as $security_key ) {
354+
if ( $security_key->keyHandle === $_POST['keyHandle'] ) { // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase
355+
$key = $security_key;
350356
break;
351357
}
352358
}
353359

354-
$key->name = $_POST['name'];
360+
if ( ! $key ) {
361+
wp_die();
362+
}
363+
364+
if ( ! isset( $_POST['name'] ) ) {
365+
wp_die();
366+
}
367+
368+
$key->name = sanitize_text_field( wp_unslash( $_POST['name'] ) );
355369

356370
$updated = Two_Factor_FIDO_U2F::update_security_key( $user_id, $key );
357371
if ( ! $updated ) {

providers/class-two-factor-totp.php

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -317,10 +317,6 @@ public static function generate_qr_code_url( $user, $secret_key ) {
317317
* @codeCoverageIgnore
318318
*/
319319
public function user_two_factor_options( $user ) {
320-
if ( ! isset( $user->ID ) ) {
321-
return;
322-
}
323-
324320
$key = $this->get_user_totp_key( $user->ID );
325321

326322
wp_enqueue_script( 'two-factor-qr-code-generator' );
@@ -690,15 +686,15 @@ public static function generate_key( $bitsize = self::DEFAULT_KEY_BIT_SIZE ) {
690686
*
691687
* @since 0.2.0
692688
*
693-
* @param string $value The value to be packed.
689+
* @param int $value The value to be packed.
694690
*
695691
* @return string Binary packed string.
696692
*/
697693
public static function pack64( $value ) {
698694
// 64bit mode (PHP_INT_SIZE == 8).
699695
if ( PHP_INT_SIZE >= 8 ) {
700696
// If we're on PHP 5.6.3+ we can use the new 64bit pack functionality.
701-
if ( version_compare( PHP_VERSION, '5.6.3', '>=' ) && PHP_INT_SIZE >= 8 ) {
697+
if ( version_compare( PHP_VERSION, '5.6.3', '>=' ) ) {
702698
return pack( 'J', $value ); // phpcs:ignore PHPCompatibility.ParameterValues.NewPackFormat.NewFormatFound
703699
}
704700
$highmap = 0xffffffff << 32;
@@ -870,7 +866,7 @@ public static function base32_encode( $string ) {
870866
$base32_string = '';
871867

872868
foreach ( $five_bit_sections as $five_bit_section ) {
873-
$base32_string .= self::$base_32_chars[ base_convert( str_pad( $five_bit_section, 5, '0' ), 2, 10 ) ];
869+
$base32_string .= self::$base_32_chars[ (int) base_convert( str_pad( $five_bit_section, 5, '0' ), 2, 10 ) ];
874870
}
875871

876872
return $base32_string;

0 commit comments

Comments
 (0)