Recently, one of our competitors in keeping track of vulnerabilities in WordPress plugins, Patchstack, very vaguely claimed there was an unfixed SQL injection vulnerability in a plugin used by at least one of our customers. As the developer noted, Patchstack didn’t “say anything specific about where the supposed vulnerability is, or how it can be reproduced.” So not all that helpful. Someone pointed out code that might be at issue. The developer didn’t take their advice in trying to fix it, leading to a new version that is still vulnerable.

What really stands out about this is the alternative suggested is much simpler than what the developer is doing instead. The issue involves properly handling inserting user input into the order by clause of an SQL statement. Here is the developer’s code before they made an attempt to fix it:

126
127
128
129
130
131
132
133
134
135
136
137
138
139
140

$req_order_by = strtolower( \esc_sql( $_REQUEST['orderby'] ?? '' ) );
$req_order    = strtoupper( \esc_sql( $_REQUEST['order'] ?? '' ) );
$order_by     = ( 0 < preg_match( '/url|hint_type|status|created_by|post_id/i', $req_order_by ) ) ? $req_order_by : '';
$order        = ( 0 < preg_match( '/ASC|DESC/', $req_order ) ) ? $req_order : '';
 
$new_query = \apply_filters( 'pprh_append_admin_sql', $query, $order_by, $order );
 
if ( $new_query === $query ) {
	if ( '' === $order_by ) {
		$order_by = 'url';
	}
	if ( '' === $order ) {
		$order = 'ASC';
	}
	$new_query['sql'] .= " ORDER BY $order_by $order";

Here is the additional code they then added:

128
129
130

if (0 < preg_match('/\s/', $req_order_by) ) {
	$req_order_by = explode(' ', $req_order_by)[0];
}

Here is the alternative suggested, which would also allow removing plenty of the existing code while actually securing this:

140
141

$order_by_sql = sanitize_sql_orderby( "{$order_by} {$order}" );
$new_query['sql'] .= " ORDER BY $order_by_sql";

WordPress provides a function to handle this used there, sanitize_sql_orderby(). It is a lot simpler to use that and if there is a problem with that, that is on WordPress. You can certainly add in additional checks when using that, but it shouldn’t be needed.

As is so often the case with security, it is better to not try to create your own security functionality. Use what already exists that has been widely used and reviewed, as that will usually be more secure than what you can come up with on your own.

Need Help Fixing a Vulnerability in Your Plugin?