Changeset 7048

Timestamp:

05/09/2013 02:11:57 PM (12 years ago)

Author:

boonebgorges

Message:

Improved sanitization in Activity component database methods

• All integer array params are filtered through wp_parse_id_list()
• Standardized LIKE clause processing

Adds tests for touched methods

Fixes #4995

Props DJPaul, johnjamesjacoby

Location:

trunk

Files:

• bp-activity/bp-activity-classes.php (modified) (5 diffs)
• tests/testcases/activity/class.BP_Activity_Activity.php (modified) (2 diffs)

trunk/bp-activity/bp-activity-classes.php

@@ -174 +174 @@
         if ( $search_terms ) {
             $search_terms = $wpdb->escape( $search_terms );
-            $where_conditions['search_sql'] = "a.content LIKE '%%" . like_escape( $search_terms ) . "%%'";
+            $where_conditions['search_sql'] = "a.content LIKE '%%" . esc_sql( like_escape( $search_terms ) ) . "%%'";
         }
 
@@ -253 +253 @@
 
         // Get the fullnames of users so we don't have to query in the loop
-        $activity_user_ids = array();
-        if ( bp_is_active( 'xprofile' ) && $activities ) {
-            foreach ( (array) $activities as $activity ) {
-                if ( (int) $activity->user_id )
-                    $activity_user_ids[] = $activity->user_id;
-            }
-
-            $activity_user_ids = implode( ',', array_unique( (array) $activity_user_ids ) );
-            if ( !empty( $activity_user_ids ) ) {
+        if ( bp_is_active( 'xprofile' ) && !empty( $activities ) ) {
+            $activity_user_ids = wp_list_pluck( $activities, 'user_id' );
+            $activity_user_ids = implode( ',', wp_parse_id_list( $activity_user_ids ) );
+
+            if ( !empty( $activity_user_ids ) ) {               
                 if ( $names = $wpdb->get_results( "SELECT user_id, value AS user_fullname FROM {$bp->profile->table_name_data} WHERE field_id = 1 AND user_id IN ({$activity_user_ids})" ) ) {
                     foreach ( (array) $names as $name )
@@ -374 +370 @@
 
         if ( !empty( $item_id ) )
-            $where_args[] = $wpdb->prepare( "item_id = %s", $item_id );
+            $where_args[] = $wpdb->prepare( "item_id = %d", $item_id );
 
         if ( !empty( $secondary_item_id ) )
-            $where_args[] = $wpdb->prepare( "secondary_item_id = %s", $secondary_item_id );
+            $where_args[] = $wpdb->prepare( "secondary_item_id = %d", $secondary_item_id );
 
         if ( !empty( $action ) )
@@ -439 +435 @@
 
         if ( !empty( $item_id ) )
-            $where_args[] = $wpdb->prepare( "item_id = %s", $item_id );
+            $where_args[] = $wpdb->prepare( "item_id = %d", $item_id );
 
         if ( !empty( $secondary_item_id ) )
-            $where_args[] = $wpdb->prepare( "secondary_item_id = %s", $secondary_item_id );
+            $where_args[] = $wpdb->prepare( "secondary_item_id = %d", $secondary_item_id );
 
         if ( !empty( $date_recorded ) )
@@ -471 +467 @@
     }
 
-    function delete_activity_item_comments( $activity_ids ) {
+    function delete_activity_item_comments( $activity_ids = array() ) {
         global $bp, $wpdb;
 
-        if ( is_array( $activity_ids ) )
-            $activity_ids = implode ( ',', array_map( 'absint', $activity_ids ) );
-        else
-            $activity_ids = implode ( ',', array_map( 'absint', explode ( ',', $activity_ids ) ) );
+        $activity_ids = implode( ',', wp_parse_id_list( $activity_ids ) );
 
         return $wpdb->query( "DELETE FROM {$bp->activity->table_name} WHERE type = 'activity_comment' AND item_id IN ({$activity_ids})" );
     }
 
-    function delete_activity_meta_entries( $activity_ids ) {
+    function delete_activity_meta_entries( $activity_ids = array() ) {
         global $bp, $wpdb;
 
-        if ( is_array( $activity_ids ) )
-            $activity_ids = implode ( ',', array_map( 'absint', $activity_ids ) );
-        else
-            $activity_ids = implode ( ',', array_map( 'absint', explode ( ',', $activity_ids ) ) );
+        $activity_ids = implode( ',', wp_parse_id_list( $activity_ids ) );
 
         return $wpdb->query( "DELETE FROM {$bp->activity->table_name_meta} WHERE activity_id IN ({$activity_ids})" );

trunk/tests/testcases/activity/class.BP_Activity_Activity.php

@@ -74 +74 @@
     }
 
-    public function test_get_meta_query() {
+    public function test_get_with_meta_query() {
         $a1 = $this->factory->activity->create();
         $a2 = $this->factory->activity->create();
@@ -90 +90 @@
         $this->assertEquals( $ids, array( $a1 ) );
     }
+
+    public function test_get_with_search_terms() {
+        $a1 = $this->factory->activity->create( array(
+            'content' => 'Boone is a cool guy',
+        ) );
+        $a2 = $this->factory->activity->create( array(
+            'content' => 'No he isn\'t',
+        ) );
+
+        $activity = BP_Activity_Activity::get( array(
+            'search_terms' => 'cool',
+        ) );
+        $ids = wp_list_pluck( $activity['activities'], 'id' );
+        $this->assertEquals( $ids, array( $a1 ) );
+    }
+
+    public function test_get_id_with_item_id() {
+        $a1 = $this->factory->activity->create( array(
+            'item_id' => 523,
+        ) );
+        $a2 = $this->factory->activity->create( array(
+            'item_id' => 1888,
+        ) );
+
+        $activity = BP_Activity_Activity::get_id( false, false, false, 523, false, false, false, false );
+        $this->assertEquals( $a1, $activity );
+    }
+
+    public function test_get_id_with_secondary_item_id() {
+        $a1 = $this->factory->activity->create( array(
+            'secondary_item_id' => 523,
+        ) );
+        $a2 = $this->factory->activity->create( array(
+            'secondary_content' => 1888,
+        ) );
+
+        $activity = BP_Activity_Activity::get_id( false, false, false, false, 523, false, false, false );
+        $this->assertEquals( $a1, $activity );
+    }
+
+    public function test_delete_with_item_id() {
+        $a1 = $this->factory->activity->create( array(
+            'item_id' => 523,
+        ) );
+        $a2 = $this->factory->activity->create( array(
+            'item_id' => 1888,
+        ) );
+
+        $activity = BP_Activity_Activity::delete( array(
+            'item_id' => 523,
+        ) );
+        $this->assertEquals( array( $a1 ), $activity );
+    }
+
+    public function test_delete_with_secondary_item_id() {
+        $a1 = $this->factory->activity->create( array(
+            'secondary_item_id' => 523,
+        ) );
+        $a2 = $this->factory->activity->create( array(
+            'secondary_item_id' => 1888,
+        ) );
+
+        $activity = BP_Activity_Activity::delete( array(
+            'secondary_item_id' => 523,
+        ) );
+        $this->assertEquals( array( $a1 ), $activity );
+    }
 }