buddypress
buddypress copied to clipboard
Add/wp unslash input sanitisation
Converting stripslashes( $_REQUEST )
and friends into wp_unslash( $_REQUEST )
, and then adding appropriate sanitization of any data.
This is a work in progress, submitted as a PR for unit test autoruns.
Trac ticket: https://buddypress.trac.wordpress.org/ticket/9065
wp_unslash conversions was mostly done with sed
without human review, this PR serves as that human review.
Input sanitization added in a few files:
- [ ] src/bp-activity/bp-activity-admin.php
- [ ] src/bp-activity/bp-activity-template.php
- [x] src/bp-blogs/bp-blogs-template.php
- [x] src/bp-core/admin/bp-core-admin-components.php
- [x] src/bp-core/admin/bp-core-admin-optouts.php
- [ ] src/bp-groups/bp-groups-admin.php
- [x] src/bp-groups/bp-groups-template.php (Needs verification still)
- [ ] src/bp-members/bp-members-template.php
- [ ] src/bp-members/classes/class-bp-members-admin.php
- [ ] src/bp-messages/bp-messages-template.php
- [ ] src/bp-notifications/bp-notifications-template.php
- [ ] src/bp-xprofile/bp-xprofile-admin.php
- [ ] src/bp-xprofile/classes/class-bp-xprofile-field-type.php
This Pull Request is for code review only. Please keep all other discussion in the BuddyPress Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the WordPress Core Handbook for more details.
As one can see... this is fairly ugly checking for the data types everywhere.
This has redirected me back to https://core.trac.wordpress.org/ticket/22325 / https://core.trac.wordpress.org/ticket/18322 and experimenting with it: https://github.com/WordPress/wordpress-develop/compare/trunk...dd32:wordpress-develop:try/request-abstraction
It might be worthwhile taking that and making a bp_get()
/ bp_get_request()
method that does this sanitisation that can later be deprecated for whatever ends up in Core.
Thanks a lot for your hard work on this @dd32 I'll review the PR asap and will try to include the fix in next minor release (12.1.0).
@imath No urgency here IMHO, this isn't worth rushing, better to get it right and clean instead.
I only did the input sanitization
on a few files in the PR, I'll add a note to the issue description for that. There's a bunch more work needed here IMHO.
Thanks for your message @dd32. I had a quick look and saw it was pretty heavy, is it ok if we fix it in next major instead of next minor (I'm always worrying about profiles.w.org)?
is it ok if we fix it in next major
Totally, this isn't a 'light' PR, nor is it urgent.
The only reason for this PR was those warnings from pentesterrs, which I realised are fatals in PHP8, this kind of thing is a "problem" for a lot of plugins, BuddyPress isn't doing something horribly wrong that needs urgent-today-attention.