Add/wp unslash input sanitisation

[dd32]

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
• src/bp-blogs/bp-blogs-template.php
• src/bp-core/admin/bp-core-admin-components.php
• src/bp-core/admin/bp-core-admin-optouts.php
• src/bp-groups/bp-groups-admin.php
• 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.

[dd32]

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: WordPress/wordpress-develop@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.

[imath]

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).

[dd32]

@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.

[imath]

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)?

[dd32]

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.

[dd32]

@imath Is there anything I can do to help this along?

For example, would smaller PRs with targeted fixes help? Such as master...dd32:buddypress:fix/search-warnings

[renatonascalves]

As part of https://buddypress.trac.wordpress.org/ticket/7228, I was looking to address some of those issues. It seems you already got started to solve some of them here.

[ ] WordPress.Security.NonceVerification.Recommended                                  519
[ ] WordPress.Security.ValidatedSanitizedInput.InputNotSanitized                      474
[ ] WordPress.Security.ValidatedSanitizedInput.MissingUnslash                         462
[ ] WordPress.Security.NonceVerification.Missing                                      248
[ ] WordPress.Security.ValidatedSanitizedInput.InputNotValidated                      128

We have quite a lot to fix here. And we need to find a way to pinpoint a commit in case a problem happens.

I'd suggest we introduce those changes per component/folder. Small pull requests where the fixes are related to a specific component only.

cc: @imath

[imath]

Hi @dd32 & @renatonascalves thanks for your comments I agree we need to progress carefully. Small SVN commits are probably the best strategy. @dd32 if you can prepare small PRs it's wonderful 😍. But we can also take advantage of this one to pick the commits & rebase it after each commit.

[renatonascalves]

@dd32 Let me know if you want me to merge/commit the current/available fixes in this pr.