Unauthenticated hidden groups disclosure via Ajax groups search

How it was found

Note: this issue was previously submitted to [email protected], because I did not have the rep to submit it here. That was cleared up with HackerOne, so I am now submitting the issue here, at @aaroncampbell's direction.

Summary

It is possible for an unauthenticated user to view the title, description, avatar, time of last activity, and membership count of hidden groups. This is possible via the Ajax groups search feature, due to a failure to properly encode user-supplied values when incorporating them into the query string within the Ajax handler. This allows the user to pass args to the groups query that they aren't intended to be able to use. Specifically, the show_hidden query arg may be used to show hidden groups.

Technical details

Groups in BuddyPress can have one of three different statuses: public, private, or hidden. Public and private groups are publicly queryable, by default, although private groups cannot be joined without an invitation. Hidden groups are not only restricted like private groups, but are also hidden in queries, unless a user has moderator capabilities.

The BP_Groups_Group::get() method is used to query for groups. Of particular interest are the status and show_hidden query args:

<?php
     if ( ! empty( $r['status'] ) ) {
        if ( ! is_array( $r['status'] ) ) {
           $r['status'] = preg_split( '/[\s,]+/', $r['status'] );
        }
        $r['status'] = array_map( 'sanitize_title', $r['status'] );
        $status_in = "'" . implode( "','", $r['status'] ) . "'";
        $where_conditions['status'] = "g.status IN ({$status_in})";
     } elseif ( empty( $r['show_hidden'] ) ) {
        $where_conditions['hidden'] = "g.status != 'hidden'";
     }

As you can see, there are no restrictions applied as to who can use these args within the method itself. So any code which allows arbitrary query args to be passed to that method by users without proper permissions is vulnerable to hidden groups disclosure.

The only place where the method is called in that manner is in its wrapper function, groups_get_groups(), which in turn is used by BP_Groups_Template to get the groups to display in a groups template. It does not accept the status query arg, but does allow show_hidden.

BP_Groups_Template::__construct() does contain this code:

<?php
     if ( bp_current_user_can( 'bp_moderate' ) || ( is_user_logged_in() && $user_id == bp_loggedin_user_id() ) ) {
        $show_hidden = true;
     }

However, note that this doesn't prevent $show_hidden from being passed in as true even if the condition isn't met. Thus, the class is still vulnerable if we can find a way to pass the show_hidden arg into it.

The groups template class is utilized only by bp_has_groups(), which is used in the groups template files to set up a groups query. The bp_has_groups() function accepts an array of query args to be passed to it, although usually that feature either isn't used or it isn't possible to pass arbitrary args in.

However, there are two places of particular interest. The first is in buddypress/bp-templates/bp-legacy/buddypress/groups/groups-loop.php on line 26:

<?php if ( bp_has_groups( bp_ajax_querystring( 'groups' ) ) ) : ?>

The value returned by the bp_ajax_querystring() function is determined via the 'bp_ajax_querystring' filter. For the Legacy template pack, the bp_legacy_theme_ajax_querystring() function is hooked to that filter to supply the value.

bp_legacy_theme_ajax_querystring() is located in buddypress/bp-templates/bp-legacy/buddypress-functions.php, and most of it is pretty well hardened against this kind of issue, properly sanitizing the user-supplied values. However, on line 692, it does this:

<?php
  // Activity stream filtering on action.
  if ( ! empty( $_BP_COOKIE['bp-' . $object . '-filter'] ) && '-1' != $_BP_COOKIE['bp-' . $object . '-filter'] ) {
     $qs[] = 'type=' . $_BP_COOKIE['bp-' . $object . '-filter'];

     // ...
  }

The value of $_BP_COOKIE can be set via $_POST['cookie'] (line 675):

<?php
  // Set up the cookies passed on this AJAX request. Store a local var to avoid conflicts.
  if ( ! empty( $_POST['cookie'] ) ) {
     $_BP_COOKIE = wp_parse_args( str_replace( '; ', '&', urldecode( $_POST['cookie'] ) ) );
  } else {
     $_BP_COOKIE = &$_COOKIE;
  }

The bp-$object-scope is also vulnerable on line 714:

<?php
  if ( ! empty( $_BP_COOKIE['bp-' . $object . '-scope'] ) ) {

     //...

     // Activity stream scope only on activity directory.
     if ( 'all' != $_BP_COOKIE['bp-' . $object . '-scope'] && ! bp_displayed_user_id() && ! bp_is_single_item() )
        $qs[] = 'scope=' . $_BP_COOKIE['bp-' . $object . '-scope'];
  }

The solution is to wrap the values in urlencode(), as is done with $_POST['search_terms'] on line 734.

<?php

     $qs[] = 'type=' . urlencode( $_BP_COOKIE['bp-' . $object . '-filter'] );

     $qs[] = 'scope=' . urlencode( $_BP_COOKIE['bp-' . $object . '-scope'] );

The BuddyPress Default theme also suffers from the same problems, in bp_dtheme_ajax_querystring() (buddypress/bp-themes/bp-default/groups/groups-loop.php uses bp_has_groups( bp_ajax_querystring( 'groups' ) ) as the Legacy template pack does). In that function, however, $_POST['search_terms'] is also a vector, since it isn't passed through urlencode() (buddypress/bp-themes/bp-default/_inc/ajax.php line 137):

<?php
  $object_search_text = bp_get_search_default_text( $object );
  if ( ! empty( $_POST['search_terms'] ) && $object_search_text != $_POST['search_terms'] && 'false' != $_POST['search_terms'] && 'undefined' != $_POST['search_terms'] )
     $qs[] = 'search_terms=' . $_POST['search_terms'];

POC

Send a POST request to /wp-admin/admin-ajax.php with the following body:

action=groups_filter&cookie=bp-groups-filter%253D%252526show_hidden%3D1&object=groups

bp-groups-filter can be replaced with bp-groups-scope to the same effect:

action=groups_filter&cookie=bp-groups-scope%253D%252526show_hidden%3D1&object=groups