Changeset 10385

Timestamp:

12/02/2015 07:49:54 PM (10 years ago)

Author:

djpaul

Message:

Improve input handling on a variety of admin screens.

An internal audit revealed some screens which react to form submission where we were not sanitising input as well as we could have been. We've added extra typecasting and existence checks to avoid mishandling causing PHP Notices.

Additionally, we moved a list of hardcoded BuddyPress "pages" (registration and account activation) into a filterable function to allow plugin developers to more easily customise or add to these, which also gives us the benefit of being able to use that function as a sanitisation whitelist, without duplicating that hardcoded list.

Props boonebgorges, DJPaul

Location:

branches/2.4/src

Files:

• bp-core/admin/bp-core-admin-slugs.php (modified) (5 diffs)
• bp-groups/bp-groups-actions.php (modified) (1 diff)
• bp-groups/bp-groups-admin.php (modified) (1 diff)

branches/2.4/src/bp-core/admin/bp-core-admin-slugs.php

@@ -40 +40 @@
 
 /**
- * Creates reusable markup for page setup on the Components and Pages dashboard panel.
- *
- * @package BuddyPress
- * @since 1.6.0
- * @todo Use settings API
- */
-function bp_core_admin_slugs_options() {
+ * Generate a list of directory pages, for use when building Components panel markup.
+ *
+ * @since 2.4.1
+ *
+ * @return array
+ */
+function bp_core_admin_get_directory_pages() {
     $bp = buddypress();
-
-    // Get the existing WP pages
-    $existing_pages = bp_core_get_directory_page_ids();
-
-    // Set up an array of components (along with component names) that have
-    // directory pages.
     $directory_pages = array();
 
@@ -69 +63 @@
     }
 
-    /** Directory Display *****************************************************/
-
     /**
      * Filters the loaded components needing directory page association to a WordPress page.
@@ -78 +70 @@
      * @param array $directory_pages Array of available components to set associations for.
      */
-    $directory_pages = apply_filters( 'bp_directory_pages', $directory_pages );
+    return apply_filters( 'bp_directory_pages', $directory_pages );
+}
+
+/**
+ * Generate a list of static pages, for use when building Components panel markup.
+ *
+ * By default, this list contains 'register' and 'activate'.
+ *
+ * @since 2.4.1
+ *
+ * @return array
+ */
+function bp_core_admin_get_static_pages() {
+    $static_pages = array(
+        'register' => __( 'Register', 'buddypress' ),
+        'activate' => __( 'Activate', 'buddypress' ),
+    );
+
+    /**
+     * Filters the default static pages for BuddyPress setup.
+     *
+     * @since 1.6.0
+     *
+     * @param array $static_pages Array of static default static pages.
+     */
+    return apply_filters( 'bp_static_pages', $static_pages );
+}
+
+/**
+ * Creates reusable markup for page setup on the Components and Pages dashboard panel.
+ *
+ * @package BuddyPress
+ * @since 1.6.0
+ * @todo Use settings API
+ */
+function bp_core_admin_slugs_options() {
+    $bp = buddypress();
+
+    // Get the existing WP pages
+    $existing_pages = bp_core_get_directory_page_ids();
+
+    // Set up an array of components (along with component names) that have directory pages.
+    $directory_pages = bp_core_admin_get_directory_pages();
+
+    /** Directory Display *****************************************************/
 
     if ( !empty( $directory_pages ) ) : ?>
@@ -141 +177 @@
     /** Static Display ********************************************************/
 
-    // Static pages
-    $static_pages = array(
-        'register' => __( 'Register', 'buddypress' ),
-        'activate' => __( 'Activate', 'buddypress' ),
-    );
-
-    /**
-     * Filters the default static pages for BuddyPress setup.
-     *
-     * @since 1.6.0
-     *
-     * @param array $static_pages Array of static default static pages.
-     */
-    $static_pages = apply_filters( 'bp_static_pages', $static_pages );
+    $static_pages = bp_core_admin_get_static_pages();
 
     if ( !empty( $static_pages ) ) : ?>
@@ -226 +249 @@
         // Then, update the directory pages
         if ( isset( $_POST['bp_pages'] ) ) {
-
-            $directory_pages = array();
-
+            $valid_pages = array_merge( bp_core_admin_get_directory_pages(), bp_core_admin_get_static_pages() );
+
+            $new_directory_pages = array();
             foreach ( (array) $_POST['bp_pages'] as $key => $value ) {
-                if ( !empty( $value ) ) {
-                    $directory_pages[$key] = (int) $value;
+                if ( isset( $valid_pages[ $key ] ) ) {
+                    $new_directory_pages[ $key ] = (int) $value;
                 }
             }
-            bp_core_update_directory_page_ids( $directory_pages );
+            bp_core_update_directory_page_ids( $new_directory_pages );
         }
 

branches/2.4/src/bp-groups/bp-groups-actions.php

@@ -210 +210 @@
                 foreach ( (array) $_POST['friends'] as $friend ) {
                     groups_invite_user( array(
-                        'user_id'  => $friend,
+                        'user_id'  => (int) $friend,
                         'group_id' => $bp->groups->new_group_id,
                     ) );

branches/2.4/src/bp-groups/bp-groups-admin.php

@@ -313 +313 @@
                 // Process only those users who have had their roles changed
                 foreach ( (array) $_POST['bp-groups-role'] as $user_id => $new_role ) {
+                    $user_id = (int) $user_id;
 
                     $existing_role = isset( $_POST['bp-groups-existing-role'][$user_id] ) ? $_POST['bp-groups-existing-role'][$user_id] : '';
 
                     if ( $existing_role != $new_role ) {
+                        $result = false;
 
                         switch ( $new_role ) {