Changeset 12507

Timestamp:

12/23/2019 07:56:32 AM (5 years ago)

Author:

imath

Message:

Improve object ID sanitization when deleting avatar or cover image

Fixes the issue in trunk.

Location:

trunk

Files:

• src/bp-core/bp-core-attachments.php (modified) (1 diff)
• src/bp-core/bp-core-avatars.php (modified) (2 diffs)
• tests/phpunit/testcases/core/avatars.php (modified) (1 diff)

trunk/src/bp-core/bp-core-attachments.php

@@ -1450 +1450 @@
     }
 
-    if ( empty( $_POST['object'] ) || empty( $_POST['item_id'] ) ) {
+    if ( empty( $_POST['object'] ) || empty( $_POST['item_id'] ) || ( ! ctype_digit( $_POST['item_id'] ) && ! is_int( $_POST['item_id'] ) ) ) {
         wp_send_json_error();
     }

trunk/src/bp-core/bp-core-avatars.php

@@ -719 +719 @@
 
     $args = wp_parse_args( $args, $defaults );
-    extract( $args, EXTR_SKIP );
 
     /**
@@ -746 +745 @@
     }
 
-    if ( empty( $item_id ) ) {
-        if ( 'user' == $object )
-            $item_id = bp_displayed_user_id();
-        elseif ( 'group' == $object )
-            $item_id = buddypress()->groups->current_group->id;
-        elseif ( 'blog' == $object )
-            $item_id = $current_blog->id;
+    if ( empty( $args['item_id'] ) ) {
+        if ( 'user' === $args['object'] ) {
+            $args['item_id'] = bp_displayed_user_id();
+        } elseif ( 'group' === $args['object'] ) {
+            $args['item_id'] = buddypress()->groups->current_group->id;
+        } elseif ( 'blog' === $args['object'] ) {
+            $args['item_id'] = $current_blog->id;
+        }
 
         /** This filter is documented in bp-core/bp-core-avatars.php */
-        $item_id = apply_filters( 'bp_core_avatar_item_id', $item_id, $object );
-
-        if ( !$item_id ) return false;
-    }
-
-    if ( empty( $avatar_dir ) ) {
-        if ( 'user' == $object )
-            $avatar_dir = 'avatars';
-        elseif ( 'group' == $object )
-            $avatar_dir = 'group-avatars';
-        elseif ( 'blog' == $object )
-            $avatar_dir = 'blog-avatars';
+        $item_id = apply_filters( 'bp_core_avatar_item_id', $args['item_id'], $args['object'] );
+    } else {
+        $item_id = $args['item_id'];
+    }
+
+    if ( $item_id && ( ctype_digit( $item_id ) || is_int( $item_id ) ) ) {
+        $item_id = (int) $item_id;
+    } else {
+        return false;
+    }
+
+    if ( empty( $args['avatar_dir'] ) ) {
+        if ( 'user' === $args['object'] ) {
+            $args['avatar_dir'] = 'avatars';
+        } elseif ( 'group' === $args['object'] ) {
+            $args['avatar_dir'] = 'group-avatars';
+        } elseif ( 'blog' === $args['object'] ) {
+            $args['avatar_dir'] = 'blog-avatars';
+        }
 
         /** This filter is documented in bp-core/bp-core-avatars.php */
-        $avatar_dir = apply_filters( 'bp_core_avatar_dir', $avatar_dir, $object );
-
-        if ( !$avatar_dir ) return false;
+        $avatar_dir = apply_filters( 'bp_core_avatar_dir', $args['avatar_dir'], $args['object'] );
+    } else {
+        $avatar_dir = $args['avatar_dir'];
+    }
+
+    if ( ! $avatar_dir ) {
+        return false;
     }
 
     /** This filter is documented in bp-core/bp-core-avatars.php */
-    $avatar_folder_dir = apply_filters( 'bp_core_avatar_folder_dir', bp_core_avatar_upload_path() . '/' . $avatar_dir . '/' . $item_id, $item_id, $object, $avatar_dir );
-
-    if ( !file_exists( $avatar_folder_dir ) )
+    $avatar_folder_dir = apply_filters( 'bp_core_avatar_folder_dir', bp_core_avatar_upload_path() . '/' . $avatar_dir . '/' . $item_id, $item_id, $args['object'], $avatar_dir );
+
+    if ( ! is_dir( $avatar_folder_dir ) ) {
         return false;
+    }
 
     if ( $av_dir = opendir( $avatar_folder_dir ) ) {
-        while ( false !== ( $avatar_file = readdir($av_dir) ) ) {
-            if ( ( preg_match( "/-bpfull/", $avatar_file ) || preg_match( "/-bpthumb/", $avatar_file ) ) && '.' != $avatar_file && '..' != $avatar_file )
+        while ( false !== ( $avatar_file = readdir( $av_dir ) ) ) {
+            if ( ( preg_match( "/-bpfull/", $avatar_file ) || preg_match( "/-bpthumb/", $avatar_file ) ) && '.' != $avatar_file && '..' != $avatar_file ) {
                 @unlink( $avatar_folder_dir . '/' . $avatar_file );
-        }
-    }
-    closedir($av_dir);
+            }
+        }
+    }
+    closedir( $av_dir );
 
     @rmdir( $avatar_folder_dir );

trunk/tests/phpunit/testcases/core/avatars.php

@@ -371 +371 @@
         return 51;
     }
+
+    /**
+     * @group bp_core_delete_existing_avatar
+     */
+    public function test_bp_core_delete_existing_avatar() {
+        $avatar_dir1 = bp_core_avatar_upload_path() . '/avatars/1';
+        $avatar_dir2 = bp_core_avatar_upload_path() . '/avatars/2';
+        wp_mkdir_p( $avatar_dir1 );
+        wp_mkdir_p( $avatar_dir2 );
+
+        copy( BP_TESTS_DIR . 'assets/upside-down.jpg', $avatar_dir1 . '/avatar-bpfull.jpg' );
+        copy( BP_TESTS_DIR . 'assets/upside-down.jpg', $avatar_dir1 . '/avatar-bpthumb.jpg' );
+        copy( BP_TESTS_DIR . 'assets/upside-down.jpg', $avatar_dir2 . '/avatar-bpfull.jpg' );
+        copy( BP_TESTS_DIR . 'assets/upside-down.jpg', $avatar_dir2 . '/avatar-bpthumb.jpg' );
+
+        $test = bp_core_delete_existing_avatar( array(
+            'item_id'    => '2/../1',
+            'object'     => 'user',
+            'avatar_dir' => false
+        ) );
+
+        $this->assertTrue( is_dir( $avatar_dir1 ) );
+
+        $test2 = bp_core_delete_existing_avatar( array(
+            'item_id'    => '2',
+            'object'     => 'user',
+            'avatar_dir' => false
+        ) );
+
+        $this->assertFalse( is_dir( $avatar_dir2 ) );
+
+        $test1 = bp_core_delete_existing_avatar( array(
+            'item_id'    => 1.1,
+            'object'     => 'user',
+            'avatar_dir' => false
+        ) );
+
+        $this->assertTrue( is_dir( $avatar_dir1 ) );
+
+        $test1 = bp_core_delete_existing_avatar( array(
+            'item_id'    => 1,
+            'object'     => 'user',
+            'avatar_dir' => false
+        ) );
+
+        $this->assertFalse( is_dir( $avatar_dir1 ) );
+    }
 }