From fc07de48b75fda09afa34d96edcb1bd81d0c0ca1 Mon Sep 17 00:00:00 2001 From: Mykyta Synelnikov Date: Wed, 28 Jun 2023 11:17:28 +0300 Subject: [PATCH] - fixed vulnerability with banned keys (made them not case-sensitive); --- includes/core/class-form.php | 20 +++++++++++-- includes/core/class-user.php | 41 ++++++++++++++++++++------- includes/core/um-actions-profile.php | 2 +- includes/core/um-actions-register.php | 2 +- 4 files changed, 50 insertions(+), 15 deletions(-) diff --git a/includes/core/class-form.php b/includes/core/class-form.php index 4b4c60a3..626047f1 100644 --- a/includes/core/class-form.php +++ b/includes/core/class-form.php @@ -390,6 +390,22 @@ if ( ! class_exists( 'um\core\Form' ) ) { } } + /** + * Remove banned wp_usermeta keys from submitted data. + * + * @since 2.6.5 + * @param array $submitted + * @return array + */ + public function clean_submitted_data( $submitted ) { + foreach ( $submitted as $metakey => $value ) { + if ( UM()->user()->is_metakey_banned( $metakey ) ) { + unset( $submitted[ $metakey ] ); + } + } + + return $submitted; + } /** * Validate form on submit @@ -478,10 +494,10 @@ if ( ! class_exists( 'um\core\Form' ) ) { // Secure sanitize of the submitted data if ( ! empty( $this->post_form ) ) { - $this->post_form = array_diff_key( $this->post_form, array_flip( UM()->user()->banned_keys ) ); + $this->post_form = $this->clean_submitted_data( $this->post_form ); } if ( ! empty( $this->post_form['submitted'] ) ) { - $this->post_form['submitted'] = array_diff_key( $this->post_form['submitted'], array_flip( UM()->user()->banned_keys ) ); + $this->post_form['submitted'] = $this->clean_submitted_data( $this->post_form['submitted'] ); } // set default role from settings on registration form diff --git a/includes/core/class-user.php b/includes/core/class-user.php index ac7a866c..b5986933 100644 --- a/includes/core/class-user.php +++ b/includes/core/class-user.php @@ -93,7 +93,7 @@ if ( ! class_exists( 'um\core\User' ) ) { 'dismissed_wp_pointers', 'session_tokens', 'screen_layout', - 'wp_user-', + $wpdb->get_blog_prefix() . 'user-', 'dismissed', 'cap_key', $wpdb->get_blog_prefix() . 'capabilities', @@ -166,9 +166,32 @@ if ( ! class_exists( 'um\core\User' ) ) { add_action( 'update_user_metadata', array( &$this, 'avoid_banned_keys' ), 10, 3 ); } + /** + * It validates the meta_key for wp_usermeta table. + * Avoid to handle `meta_key` by the UM Forms if it contains $this->banned_keys inside. + * + * @since 2.6.5 + * + * @param string $meta_key Usermeta key. + * @return bool + */ + public function is_metakey_banned( $meta_key ) { + $is_banned = false; + foreach ( $this->banned_keys as $ban ) { + if ( is_numeric( $meta_key ) || false !== stripos( $meta_key, $ban ) ) { + $is_banned = true; + break; + } + } + + return $is_banned; + } + /** * Low-level checking to avoid updating banned user metakeys while UM Forms submission. * + * @since 2.6.4 + * * @param null|bool $check Whether to allow updating metadata for the given type. * @param int $object_id ID of the object metadata is for. * @param string $meta_key Metadata key. @@ -180,7 +203,7 @@ if ( ! class_exists( 'um\core\User' ) ) { return $check; } - if ( in_array( $meta_key, $this->banned_keys, true ) ) { + if ( $this->is_metakey_banned( $meta_key ) ) { $check = false; } @@ -1299,21 +1322,17 @@ if ( ! class_exists( 'um\core\User' ) ) { $this->set(0, $clean); } - /** - * Clean user profile + * Clean user profile before set. */ - function clean() { + private function clean() { foreach ( $this->profile as $key => $value ) { - foreach ( $this->banned_keys as $ban ) { - if ( strstr( $key, $ban ) || is_numeric( $key ) ) { - unset( $this->profile[ $key ] ); - } + if ( $this->is_metakey_banned( $key ) ) { + unset( $this->profile[ $key ] ); } } } - /** * This method lets you auto sign-in a user to your site. * @@ -2160,7 +2179,7 @@ if ( ! class_exists( 'um\core\User' ) ) { $changes = apply_filters( 'um_before_update_profile', $changes, $args['ID'] ); foreach ( $changes as $key => $value ) { - if ( in_array( $key, $this->banned_keys, true ) ) { + if ( $this->is_metakey_banned( $key ) ) { continue; } diff --git a/includes/core/um-actions-profile.php b/includes/core/um-actions-profile.php index f5cd85a0..e7a6dd36 100644 --- a/includes/core/um-actions-profile.php +++ b/includes/core/um-actions-profile.php @@ -1557,7 +1557,7 @@ function um_submit_form_profile( $args ) { UM()->fields()->editing = true; if ( ! empty( $args['submitted'] ) ) { - $args['submitted'] = array_diff_key( $args['submitted'], array_flip( UM()->user()->banned_keys ) ); + $args['submitted'] = UM()->form()->clean_submitted_data( $args['submitted'] ); } /** diff --git a/includes/core/um-actions-register.php b/includes/core/um-actions-register.php index 793feb16..0d79bbcd 100644 --- a/includes/core/um-actions-register.php +++ b/includes/core/um-actions-register.php @@ -412,7 +412,7 @@ function um_submit_form_register( $args ) { ); if ( ! empty( $args['submitted'] ) ) { - $args['submitted'] = array_diff_key( $args['submitted'], array_flip( UM()->user()->banned_keys ) ); + $args['submitted'] = UM()->form()->clean_submitted_data( $args['submitted'] ); } $args['submitted'] = array_merge( $args['submitted'], $credentials );