20 Jul

WordPress Plugin Developers Don’t Always Have the Best Understanding of the Security of Their Plugins

Recently on a fairly regular basis we have been interacting with developers of WordPress plugins that seem to believe that they have a better understanding of security than they actually do, which has lead to them either not fixing vulnerabilities or not making security improvements to their plugins that they really should be. What makes dealing with that more irritating is when we see developers making changes to fix vulnerabilities that don’t actually exist in their plugins. So real vulnerabilities are not getting fixed, while non vulnerabilities are getting “fixed”. Usually that “fix” involves the developer duplicating security protection already being handled in the plugin or by WordPress. Other than causing the plugin to use slightly more resources than needed, this isn’t usually detrimental. If there is someone to fault for that, it is those making false claims about vulnerabilities in plugins, which occurs all too often.

As an example of an unneeded fix, let’s take a look at version 5.2.13 of bbPress. The release announcement states that the version:

adds some sanitization to anonymous user data that went missing from previous versions

and

If your site allows anonymous users (users without registered accounts) to create topics & replies in your forums, you’ll want to upgrade to 2.5.13 right away.

Looking at the changes made in that version you can see that in the file /includes/common/functions.php several values are now run through sanitization functions.

Here are how the important values looked as of version 2.5.12 (the other value is derived from one of these):

615
616
617
618
619
$r = bbp_parse_args( $args, array (
	'bbp_anonymous_name'    => !empty( $_POST['bbp_anonymous_name']    ) ? $_POST['bbp_anonymous_name']    : false,
	'bbp_anonymous_email'   => !empty( $_POST['bbp_anonymous_email']   ) ? $_POST['bbp_anonymous_email']   : false,
	'bbp_anonymous_website' => !empty( $_POST['bbp_anonymous_website'] ) ? $_POST['bbp_anonymous_website'] : false,
), 'filter_anonymous_post_data' );

There you have three POST inputs being brought in without being sanitized, but the next lines down then do the sanitization:

621
622
623
624
625
626
627
628
629
630
631
// Filter variables and add errors if necessary
$r['bbp_anonymous_name'] = apply_filters( 'bbp_pre_anonymous_post_author_name',  $r['bbp_anonymous_name']  );
if ( empty( $r['bbp_anonymous_name'] ) )
	bbp_add_error( 'bbp_anonymous_name',  __( '<strong>ERROR</strong>: Invalid author name submitted!',   'bbpress' ) );
 
$r['bbp_anonymous_email'] = apply_filters( 'bbp_pre_anonymous_post_author_email', $r['bbp_anonymous_email'] );
if ( empty( $r['bbp_anonymous_email'] ) )
	bbp_add_error( 'bbp_anonymous_email', __( '<strong>ERROR</strong>: Invalid email address submitted!', 'bbpress' ) );
 
// Website is optional
$r['bbp_anonymous_website'] = apply_filters( 'bbp_pre_anonymous_post_author_website', $r['bbp_anonymous_website'] );

That code calls filtering defined in the file /includes/core/filters.php:

214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
/**
 * Add filters to anonymous post author data
 */
// Post author name
add_filter( 'bbp_pre_anonymous_post_author_name',    'trim',                10 );
add_filter( 'bbp_pre_anonymous_post_author_name',    'sanitize_text_field', 10 );
add_filter( 'bbp_pre_anonymous_post_author_name',    'wp_filter_kses',      10 );
add_filter( 'bbp_pre_anonymous_post_author_name',    '_wp_specialchars',    30 );
 
// Save email
add_filter( 'bbp_pre_anonymous_post_author_email',   'trim',                10 );
add_filter( 'bbp_pre_anonymous_post_author_email',   'sanitize_email',      10 );
add_filter( 'bbp_pre_anonymous_post_author_email',   'wp_filter_kses',      10 );
 
// Save URL
add_filter( 'bbp_pre_anonymous_post_author_website', 'trim',                10 );
add_filter( 'bbp_pre_anonymous_post_author_website', 'wp_strip_all_tags',   10 );
add_filter( 'bbp_pre_anonymous_post_author_website', 'esc_url_raw',         10 );
add_filter( 'bbp_pre_anonymous_post_author_website', 'wp_filter_kses',      10 );

For the author name and email field one the filters applied is the same as the sanitization added in version 5.2.13, a different one is applied to the URL:

615
616
617
618
619
$r = bbp_parse_args( $args, array (
	'bbp_anonymous_name'    =&gt; !empty( $_POST['bbp_anonymous_name']    ) ? sanitize_text_field( $_POST['bbp_anonymous_name']    ) : false,
	'bbp_anonymous_email'   =&gt; !empty( $_POST['bbp_anonymous_email']   ) ? sanitize_email(      $_POST['bbp_anonymous_email']   ) : false,
	'bbp_anonymous_website' =&gt; !empty( $_POST['bbp_anonymous_website'] ) ? sanitize_text_field( $_POST['bbp_anonymous_website'] ) : false,
), 'filter_anonymous_post_data' );

One of the reasons it is important get be accurate with claims that a new version fixes a vulnerability is something else mentioned in the release announcement:

Also, remember that since bbPress 2.5.12, the minimum WordPress version allowed is 4.7. If you need to user a previous version of WordPress, you will want to continue to use 2.5.11.

If you use our service we have checked on claims that a new version fixes a vulnerability before adding it our data set, so you will know that if we are indicating that a version is vulnerable than it actually is. And if see something were not listing in the data shown on the service’s companion plugin’s page (and haven’t sent you an email if you are using an impacted version) you can get in touch with us to see why that is.

One thought on “WordPress Plugin Developers Don’t Always Have the Best Understanding of the Security of Their Plugins

  1. I think in general it is better when the sanitization is more obvious, but there is no reason to do it twice. I would probably never rely just on a filter to provide sanitization in this way, because it is so easy to overlook. I’d say it is a poor design, and it leads to developers not knowing whether something is secure or not. Early sanitization and late escaping make it obvious whether something is safe or not.

Leave a Reply

Your email address will not be published. Required fields are marked *