Skip to:
Content

BuddyPress.org

Opened 9 years ago

Closed 9 years ago

#6941 closed defect (bug) (fixed)

Make sure inviter_id !== 0 before inviting a user to a group

Reported by: danbrellis's profile danbrellis Owned by: dcavins's profile dcavins
Milestone: 2.6 Priority: high
Severity: normal Version:
Component: Groups Keywords: needs-patch good-first-bug
Cc:

Description

Because of some errors on my end with checking permissions, my members got hundreds of spam invitations to every group on the site. I should have had the proper systems in place to prevent this, but while i was investigating I came across something in core that would have prevented this also.

In groups_invite_user() (line 1090 of bp-groups/bp-groups-functions.php), we should check if the inviter id is not 0. In my case, all of the spam invites had an inviter_id of 0.

It could be as simple as ammending line 1103:

if ( empty( $user_id ) || empty( $group_id ) || empty( $inviter_id) )
  return false;

If I'm wrong in assuming that every invitation should have an inviter, I apologize for jumping to conclusions. Also, I'm sure there are other methods in place to correct this, but when someone is developing a plugin (like I was) that uses this function, it would be a nice back-up. Especially since in the query for invites in BP_Groups_Member::get_invites(), the statement includes

AND m.inviter_id != 0

Change History (6)

#1 @dcavins
9 years ago

  • Owner set to dcavins
  • Status changed from new to accepted

Thanks very much for your report. Can you clarify a few things:

Were these invitations generated during group creation, or were they invitations to an existing group?
Are you using BP's built-in invitation functionality, or are you using the plugin Invite Anyone or any other extension?

Thanks!

#2 @dcavins
9 years ago

Hi @danbrellis-

I should also mention that, in the groups_members table, membership requests to a group are recorded with an inviter_id of 0. So maybe that's what you're seeing?

The reason the check doesn't currently exist in groups_invite_user() is that calls to it only occur on access-protected pages and after a nonce check (so the user had to come from an access-protected page). We're assuming that bp_loggedin_user_id() will always be true in those circumstances.

That said, I'm fine with making the change you're requesting, though.

Thanks!

#3 @danbrellis
9 years ago

These were group invitations to existing groups. The calls were made through a custom script (triggered by a $_GET variable) I had written that uses groups_invite_user() and while I did check for a nonce, I didn't check to make sure bp_loggedin_user_id() didn't return 0. Hence, something/someone was able to bypass or replicate the nonce and make the call to the script even though they were not logged in.

To your other point about membership requests to a group being recorded with an inviter_id of 0, I didn't know that. However, I do know that all of these calls were invites and not requests because I had complaints from my users about receiving dozens of emails that they were invited to all these groups.

PS- I am using a custom script and not BP's built in invite functionality to call groups_invite_user() because my site doesn't utilize the friends component.

Like I said, I figured BP already had checks in place and I learned my lesson abut doing it myself, but if that added level of security doesn't interfere with any normal operations, I would suggest adding it in since it's simple enough and might help someone else out.

Thanks for the time.

#4 @DJPaul
9 years ago

  • Keywords needs-patch good-first-bug added
  • Milestone changed from Awaiting Review to 2.6
  • Priority changed from normal to high
  • Version 2.5.0 deleted

Yes, definitely. Anything we are going to store in the database we must check the value is set, otherwise we'll end up with more 0 weirdness or worse. :)

Thanks for letting us know @danbrellis

This ticket was mentioned in Slack in #buddypress by dcavins. View the logs.


9 years ago

#6 @dcavins
9 years ago

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

In 10687:

Make sure invites include an inviter_id.

  • When creating an invite, make sure that the

inviter_id is not 0.

  • Add tests for empty values for user_id,

group_id and inviter_id in
groups_send_invites().

Props danbrellis.

Fixes #6941.

Note: See TracTickets for help on using tickets.