WordPress / plugin-check

A repository for the new Plugin Check plugin from the WordPress Performance and Plugins Team.
https://wordpress.org/plugins/plugin-check/
GNU General Public License v2.0
266 stars 53 forks source link

"Security" warnings about unescaped output #802

Open joho1968 opened 3 days ago

joho1968 commented 3 days ago

Sorry about nagging about this, but I quite often fail to see how this "escaping output" thing is supposed to work in an actual scenario.

// Mimic class-IXR-server output
$charset = function_exists( 'get_option' ) ? get_option( 'blog_charset' ) : '';
if ( ! empty( $charset ) ) {
    $xml_safe = '<?xml version="1.0" encoding="' . wp_kses_no_null( $charset ) . '"?>' . "\n";
} else {
    $xml_safe = '<?xml version="1.0"?>' . "\n";
}
header( 'Connection: close' );
if ( ! empty( $charset ) ) {
    header( 'Content-Type: text/xml; charset=' . wp_kses_no_null( $charset ) );
} else {
    header( 'Content-Type: text/xml' );
}
header( 'Date: ' . gmdate( 'r' ) );
echo $xml_safe;
echo wp_kses_no_null( $error->getXml() );

This generates warning to the effect of

ERROR   
WordPress.Security.EscapeOutput.OutputNotEscaped    
All output should be run through an escaping function (see the Security sections in the WordPress Developer Handbooks), found '$xml_safe'.

ERROR   
WordPress.Security.EscapeOutput.OutputNotEscaped    
All output should be run through an escaping function (see the Security sections in the WordPress Developer Handbooks), found 'wp_kses_no_null'. 

Why? And what, more precisely, am I supposed to use here ...? 🤔

The same thing applies when I create a concatenated string containing HTML for output like:

$html = '<div>This and that</div>';
$html .= '<p>Here is more output</p>';
echo $html;

This will also generate "security warnings" ...

davidperezgar commented 2 days ago

I suggest to instead of using a variable that gets all data, echo every line so it will be escaped in all lines.

joho1968 commented 2 days ago

I suggest to instead of using a variable that gets all data, echo every line so it will be escaped in all lines.

That is, of course, an option. And maybe I need to re-think this specifically for WordPress coding... 🤔