[wp-trac] [WordPress Trac] #56434: Check that the input is a string in wp_strip_all_tags()
WordPress Trac
noreply at wordpress.org
Sun Aug 28 22:58:01 UTC 2022
#56434: Check that the input is a string in wp_strip_all_tags()
-------------------------------------------------+-------------------------
Reporter: chocofc1 | Owner: (none)
Type: defect (bug) | Status: new
Priority: normal | Milestone: 6.1
Component: Formatting | Version: 2.9
Severity: minor | Resolution:
Keywords: has-patch has-unit-tests php81 2nd- | Focuses:
opinion |
-------------------------------------------------+-------------------------
Comment (by peterwilsoncc):
Replying to [comment:5 jrf]:
> Leaving the `_doing_it_wrong()` out, as @peterwilsoncc
[https://github.com/WordPress/wordpress-
develop/pull/3132?#discussion_r954384359 suggests], would be hiding an
error which should be fixed instead, which is bad practice and not helping
devs.
I disagree:
* WordPress needs to make sanitization of data as easy as possible. Rather
than assisting site owners and developers here, logging a notice here
would add noise for a security function that is handling the use case
* the WPCS security sniffs recommend unslashing and an `isset`/`empty`
check. They do not recommend a type check.
Adding a notice would require developers change change the code:
{{{#!php
<?php
$thing = ! empty( $_POST['t'] ) ? wp_strip_all_tags( wp_unslash(
$_POST['t'] ) ) : '';
}}}
to
{{{#!php
<?php
if ( ! empty( $POST['t'] ) && is_string( $POST['t'] ) {
$thing = wp_strip_all_tags( wp_unslash( $_POST['t'] ) );
} else {
$this = ''; // or false or whatever works.
}
}}}
An alarming number of extenders already fail to sanitize correctly, making
it more difficult for them to do so is not going to help improve the
situation.
--
Ticket URL: <https://core.trac.wordpress.org/ticket/56434#comment:8>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list