PHP 7.1 Stops Some Improper Usage of wpdb::prepare() Function
Unlike any other data sources on vulnerabilities in WordPress plugins that we are aware of, we actually test out claimed vulnerabilities when adding them to our data set (though as Wordfence shows, people will lie about doing that sort of thing). That involves a fair amount of work, but it provides much better results as other data sources will falsely claim that vulnerabilities that haven’t been fixed have been fixed and includes false reports of vulnerabilities. One issue that has been coming up on a more frequent basis recently when doing that testing has been dealing with issues that vary with the test environment.
We recently were rechecking a plugin to see if a new version had fixed a vulnerability and at first it looked like it had, but in reality it turned out that with the Gutenberg editor enabled, the plugin’s input fields were not being saved, so at first it looked like malicious code was properly being removed, but upon further testing we realized that the input were not being saved at all. When using the Classic editor the malicious code would still be saved.
Another issue came up when testing out a change made to the plugin WP Google Maps. The changelog for version 7.11.18 is “Fixed potential REST API exploit (affects 7.11.00 – 7.11.17 – with thanks to Thomas Chauchefoin)” and the refers to a SQL injection issue.
When testing things out we ran into this fatal error:
uncaught ArgumentCountError: Too few arguments to function wpdb::prepare()
Here is the code that caused that:
151 | $stmt = $wpdb->prepare("SELECT $imploded FROM $wpgmza_tblname"); |
That function requires at least two arguments, as there should be SQL statement and then at least one additional argument with data to replace the placeholder(s) in the SQL statement. Without that second argument, nothing is actually being prepared there.
A fatal error seems like it should have been noticed before even the SQL injection issue was noticed, but a check shows that is something that only occurs as of PHP 7.1. Here is how the PHP documentation explains that:
Previously, a warning would be emitted for invoking user-defined functions with too few arguments. Now, this warning has been promoted to an Error exception. This change only applies to user-defined functions, not internal functions.
That turns out to not be the only coding problem in that code as code that looks like it was intended to limit the values that could be passed into a SQL statement seems to have been done backwards, as there was previously this code:
146 147 | foreach($fields as $key => $value) $fields[$key] = '`' . preg_replace('/[a-z_]/i', '', $value) . '`'; |
That code removes usage of letters and underscores from values in an array variable $fields. With the new code, it will limit the values that can be included in that array to as of the current version:
- id
- map_id
- address
- description
- pic
- link
- icon
- lat
- lng
- anim
- title
- infoopen
- category
- approved
- retina
- type
- did
- other_data
- latlng
Which all would have been removed with the old one, so it seems the intent was actually to remove any other characters. That seems like it would limit the possibility of accomplishing SQL injection as you could not use any of those characters.
With that level of mistakes it maybe isn’t surprising that we have found that the plugin currently contains a very serious exploitable vulnerability.