Skip to:
Content

BuddyPress.org

Opened 5 years ago

Last modified 5 years ago

#5664 new defect (bug)

BP setup routine can run erroneously

Reported by: boonebgorges Owned by:
Milestone: Awaiting Contributions Priority: normal
Severity: major Version:
Component: Core Keywords:
Cc:

Description

A few days back I got an emergency ticket from a client, letting me know that the site was whitescreening. After some debugging, it turned out that a number of components had been deactivated (groups, legacy forums, messages), causing fatal errors with custom functionality. Some more investigation showed that the existing directory pages for these components had also been deleted (along with the Register and Activity pages), with multiple versions of these pages regenerated in their place, and the Dashboard > BuddyPress > Pages settings switched to point to the most recently created versions (so the URLs were members-3, etc). The page authors were three different non-admin users, and the pages had all been created at the exact same time.

It's hard to know for sure, but my intepretation is that the BP installation routine was being run incorrectly. See the if ( bp_is_install() ) block here https://buddypress.trac.wordpress.org/browser/tags/2.0.1/bp-core/bp-core-update.php#L199: keeping in mind that the default components are Members, Activity, and XProfile, running this code on a production site would result in just the symptoms described above.

How could this happen? A possible trace:

  • bp_is_install() is true if ! bp_get_db_version_raw(). ! bp_get_db_version_raw() is true if $bp->db_version_raw is empty(), which would presumably happen here https://buddypress.trac.wordpress.org/browser/tags/2.0.1/bp-loader.php#L503. I'm not sure how or why this would happen, but it does explain how the bp_is_install() check would pass.
  • There is also the question of how we got to the bp_version_updater() function to begin with. It is called by bp_setup_updater(), but only when bp_is_update() is true; but bp_is_update() will return true under similar circumstances as bp_is_install(), described above. bp_setup_updater() is hooked to 'bp_admin_init', which suggests that the entire episode was triggered by these non-admin users visiting a wp-admin page at a moment when something was messing with the get_site_option( '_bp_db_version' ) call or something like that.

I'm afraid I don't have any more details, or any concrete ideas about how to trace it further. Part of the purpose of this ticket is to ask whether anyone here has ever seen anything like this, or has any ideas about where else I could look.

The second reason for opening the ticket is to suggest that there could be more safeguards in place to prevent this sort of problem from happening. A couple thoughts:

  1. Bail out of bp_version_updater() based on a cap ('bp_moderate' or maybe something stronger) check. This may not have prevented the problem, but it would at least prevent the odd situation of directory pages being generated and being attributed to a non-admin user.
  2. In bp_version_updater(), do some sort of gentle checks before wiping out existing 'bp-active-components' settings and directory pages. Another way of putting this: do we want the absence of db_version_raw, all by itself, to trigger the deletion of existing content? Seems to me we should use data if we find it there; users who want to trigger a full reinstall by deleting _bp_db_version will be savvy enough to delete bp_pages and bp-active-components too.

Any thoughts are welcome.

Change History (1)

#1 @DJPaul
5 years ago

  • Milestone changed from Awaiting Review to Future Release

Sounds pretty hairy. Moving to FR while thinking about this for a bit more.

Note: See TracTickets for help on using tickets.