Opened 10 years ago
Closed 4 months ago
#5664 closed defect (bug) (worksforme)
BP setup routine can run erroneously
Reported by: | boonebgorges | Owned by: | |
---|---|---|---|
Milestone: | 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
isempty()
, 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 thebp_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 bybp_setup_updater()
, but only whenbp_is_update()
is true; butbp_is_update()
will return true under similar circumstances asbp_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 awp-admin
page at a moment when something was messing with theget_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:
- 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. - 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 ofdb_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 deletebp_pages
andbp-active-components
too.
Any thoughts are welcome.
Sounds pretty hairy. Moving to FR while thinking about this for a bit more.