Skip to content

Commit 7171654

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 933fd5a commit 7171654

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
@@ -907,7 +907,7 @@ public static function is_api_request() {
907907
*
908908
* @since 0.2.0
909909
*
910-
* @param WP_User $user WP_User object of the logged-in user.
910+
* @param WP_User|false $user WP_User object of the logged-in user.
911911
*/
912912
public static function show_two_factor_login( $user ) {
913913
if ( ! $user ) {
@@ -1750,9 +1750,9 @@ public static function _login_form_revalidate_2fa( $nonce = '', $provider = '',
17501750
*
17511751
* @since 0.9.0
17521752
*
1753-
* @param object $provider The Two Factor Provider.
1754-
* @param WP_User $user The user being authenticated.
1755-
* @param bool $is_post_request Whether the request is a POST request.
1753+
* @param object|null $provider The Two Factor Provider.
1754+
* @param WP_User $user The user being authenticated.
1755+
* @param bool $is_post_request Whether the request is a POST request.
17561756
* @return false|WP_Error|true WP_Error when an error occurs, true when the user is authenticated, false if no action occurred.
17571757
*/
17581758
public static function process_provider( $provider, $user, $is_post_request ) {
@@ -2059,7 +2059,7 @@ public static function user_two_factor_options( $user ) {
20592059
<h2><?php esc_html_e( 'Two-Factor Options', 'two-factor' ); ?></h2>
20602060

20612061
<?php foreach ( $notices as $notice_type => $notice ) : ?>
2062-
<div class="<?php echo esc_attr( $notice_type ? 'notice inline notice-' . $notice_type : '' ); ?>">
2062+
<div class="<?php echo esc_attr( 'notice inline notice-' . $notice_type ); ?>">
20632063
<p><?php echo wp_kses_post( $notice ); ?></p>
20642064
</div>
20652065
<?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
@@ -335,7 +335,7 @@ public function generate_and_email_token( $user ) {
335335
*
336336
* @since 0.1-dev
337337
*
338-
* @param WP_User $user WP_User object of the logged-in user.
338+
* @param WP_User|false $user WP_User object of the logged-in user.
339339
*/
340340
public function authentication_page( $user ) {
341341
if ( ! $user ) {
@@ -395,7 +395,7 @@ public function authentication_page( $user ) {
395395
* @return boolean
396396
*/
397397
public function pre_process_authentication( $user ) {
398-
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.
398+
if ( isset( $_REQUEST[ self::INPUT_NAME_RESEND_CODE ] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- non-distructive option that relies on user state.
399399
$this->generate_and_email_token( $user );
400400
return true;
401401
}
@@ -413,7 +413,7 @@ public function pre_process_authentication( $user ) {
413413
*/
414414
public function validate_authentication( $user ) {
415415
$code = $this->sanitize_code_from_request( 'two-factor-email-code' );
416-
if ( ! isset( $user->ID ) || ! $code ) {
416+
if ( ! $code ) {
417417
return false;
418418
}
419419

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
@@ -326,10 +326,6 @@ public static function generate_qr_code_url( $user, $secret_key ) {
326326
* @codeCoverageIgnore
327327
*/
328328
public function user_two_factor_options( $user ) {
329-
if ( ! isset( $user->ID ) ) {
330-
return;
331-
}
332-
333329
$key = $this->get_user_totp_key( $user->ID );
334330

335331
wp_enqueue_script( 'two-factor-qr-code-generator' );
@@ -720,11 +716,11 @@ public static function pack64( int $value ): string {
720716
if ( 8 === PHP_INT_SIZE ) {
721717
return pack( 'J', $value );
722718
}
723-
719+
724720
// 32-bit PHP fallback
725721
$higher = ( $value >> 32 ) & 0xFFFFFFFF;
726722
$lower = $value & 0xFFFFFFFF;
727-
723+
728724
return pack( 'NN', $higher, $lower );
729725
}
730726

@@ -890,7 +886,7 @@ public static function base32_encode( $string ) {
890886
$base32_string = '';
891887

892888
foreach ( $five_bit_sections as $five_bit_section ) {
893-
$base32_string .= self::$base_32_chars[ base_convert( str_pad( $five_bit_section, 5, '0' ), 2, 10 ) ];
889+
$base32_string .= self::$base_32_chars[ (int) base_convert( str_pad( $five_bit_section, 5, '0' ), 2, 10 ) ];
894890
}
895891

896892
return $base32_string;

0 commit comments

Comments
 (0)