Skip to:
Content

BuddyPress.org

Opened 7 years ago

Closed 7 years ago

#5478 closed defect (bug) (fixed)

Activity items can be favorited multiple times by the same user

Reported by: mpa4hu Owned by: boonebgorges
Milestone: 2.0 Priority: high
Severity: normal Version:
Component: Activity Keywords:
Cc: mpa4hu@…

Description

This is not much security issue but interesting workaround.

var type = target.hasClass('fav') ? 'fav' : 'unfav';
var parent = target.closest('.activity-item');
var parent_id = parent.attr('id').substr( 9, parent.attr('id').length );
target.addClass('loading');
jq.post( ajaxurl, {

action: 'activity_mark_' + type,
'cookie': bp_get_cookies(),
'id': parent_id

}

this is a client side script that handles favoriting activity.

Then on server side when removing activity favorite (bp_activity_remove_user_favorite) you check
$my_favs = array_unique( array_flip( $my_favs ) );
array_unique (i think) guarantees that array meta data stays clean.

On the other hand when adding activity as favorite (bp_activity_add_user_favorite) There is no such thing.

at first this might not sound serious, but for example I have a filter thats orders activities by favorite_count meta. and since there is no check from backend if its already favorited or not, I can increment that meta as many times as I want.

Change History (5)

#1 @boonebgorges
7 years ago

  • Component changed from Core to Activity
  • Milestone changed from Awaiting Review to 2.0
  • Summary changed from Security while favorting activity to Activity items can be favorited multiple times by the same user

Good catch.

#2 @boonebgorges
7 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 8175:

Don't allow a user to favorite a single activity item more than once.

Fixes #5478

#3 @mpa4hu
7 years ago

what about bp_activity_remove_user_favorite
array_unique will guarantee that user_meta stays clean, but I still can decrement favorite_count meta

#4 @boonebgorges
7 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#5 @boonebgorges
7 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 8177:

Bail from bp_activity_remove_user_favorite() if the user has not previously favorited the item in question

The failure to do this check could result in the incorrect decrementing of the
item's favorite_count.

Fixes #5478

Note: See TracTickets for help on using tickets.