Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make WordPressVIPMinimum.Security.PHPFilterFunctions.restricted_filters configurable #753

Open
gmazzap opened this issue Mar 16, 2023 · 0 comments

Comments

@gmazzap
Copy link

gmazzap commented Mar 16, 2023

What problem would the enhancement address for VIP?

In PHP 8.1 the filter FILTER_SANITIZE_STRING has been deprecated.

So a code that was using:

$x = filter_input(INPUT_POST, 'something', FILTER_SANITIZE_STRING);

now must use:

$x = wp_strips_all_tags(filter_input(INPUT_POST, 'something') ?: '');

But this is reported as an error by WordPressVIPMinimum.Security.PHPFilterFunctions.

Even if the code becomes:

$x = wp_strips_all_tags(filter_input(INPUT_POST, 'something', FILTER_UNSAFE_RAW) ?: '');

The sniff still reports the error, because FILTER_UNSAFE_RAW is marked as a "restricted filter" here: https://github.com/Automattic/VIP-Coding-Standards/blob/develop/WordPressVIPMinimum/Sniffs/Security/PHPFilterFunctionsSniff.php#L48-L51

It means that when a developer escape the string after using filter_input, there's no way to not trigger an error, because there's no filter constant that replaces FILTER_SANITIZE_STRING, and they are forced to ignore the rule.

I think that if a developer purposely write FILTER_UNSAFE_RAW they are aware the value they get is unescaped and so they need to escape later.

Describe the solution you'd like

I'd like PHPFilterFunctionsSniff.php's $restricted_filters property to be public and so configurable. In that case I could remove FILTER_UNSAFE_RAW from restricted filters.

The change would be 100% backward compatible, because unless someone changes the configuration there's no change in behavior.

What code should be reported as a violation?

No change, unless sniff configuration is changed. And if config changes what's reported depends on configuration.

What code should not be reported as a violation?

No change, unless sniff configuration is changed. And if config changes what's not reported depends on configuration.

Additional context

--

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant