02 May

Did Sucuri Lie About a Claimed SQL Injection Vulnerability or Unnecessarily Frighten People Due to Not Doing Basic Testing?

Yesterday we wrote about the web security company Sucuri overstating the impact of a SQL injection vulnerability, which they had re-discovered a year and half after we had discussed it. That was one of two claimed SQL injection vulnerabilities they disclosed recently and the post on the other, claimed to be in the plugin Advance Contact Form 7 DB, manages to be worse.

Their post starts by making a claim that doesn’t seem to make sense:

As part of our regular research audits for our Sucuri Firewall, we discovered an SQL injection vulnerability affecting 40,000+ users of the Advanced Contact Form 7 DB WordPress plugin.

Reviewing the security of a plugin for SQL injection vulnerabilities shouldn’t be connected to an audit for a firewall, it seems the intent is probably to advertise their firewall, which makes the rest of what we found seem worse.

They gave the claimed vulnerability a DREAD Score of 7/10, which is not all accurate when you know what we found.

Later in the post they make this claim:

If attackers are able to set arbitrary values for the variable $fid, they can modify the query in such a way that can lead to a full server compromise. We cannot underestimate the ramifications of an SQL injection — or know exactly how many servers may allow attackers to obtain more than just encrypted hashes and user emails.

Looking at the code impacted tells a different story. There are two SQL statements that are not properly secured.

Here is the one of them:

194
$arr_total = $wpdb->get_results("SELECT data_id FROM `".VSZ_CF7_DATA_ENTRY_TABLE_NAME."` WHERE `cf7_id` = " .$fid . " ".((!empty($search)) ? "AND `value` LIKE '%%".$search."%%'" : "")." GROUP BY `data_id`");

Here is the second:

185
186
187
188
189
190
191
$query = "SELECT * FROM `".VSZ_CF7_DATA_ENTRY_TABLE_NAME."` WHERE `cf7_id` = ".$fid." AND data_id IN(
			SELECT * FROM (
				SELECT data_id FROM `".VSZ_CF7_DATA_ENTRY_TABLE_NAME."` WHERE 1 = 1 AND `cf7_id` = ".$fid. ((!empty($search)) ? " AND `value` LIKE '%%".$search."%%'" : "") . ((!empty($search_date_query)) ? $search_date_query : "") ." 
					GROUP BY `data_id` ORDER BY ".$cf7d_entry_order_by." LIMIT ".$offset.",".$items_per_page."
				) 
			temp_table) 
			ORDER BY " . $cf7d_entry_order_by;
197
$data = $wpdb->get_results($query);

What is important about those is that they involve SELECT statements, so what you could do with those is limited to getting data from the database. That doesn’t match the next part of their post as they make this claim:

While plugin users won’t necessarily see anything malicious on the website, the most obvious indicator of compromise would be malicious content found within the database.

Some SQL injection vulnerabilities allow modifying data in the database, but just looking at that code would tell that wasn’t the case here. What looks to be there based on that code is a vulnerability that as we mentioned in yesterday’s post is not likely to be exploited on the average website.

Confusingly after that it is then stated:

SQL injections are dangerous and can impact a variety of SQL databases. Using this vector, attackers can manipulate a site argument in order to inject their own commands to the database and retrieve sensitive data — including usernames and password hashes.

All of what we just told you there, though, doesn’t really matter, since you can’t actually exploit this as Sucuri makes it sound. They may have given a good clue as to that when they wrote this:

If attackers are able to set arbitrary values for the variable $fid, they can modify the query in such a way that can lead to a full server compromise.

Nowhere in the post do they actually state that attackers can do that, which either is because Sucuri didn’t fully look through things or they realized the truth of the situation and didn’t want that to get in the way in their advertisement for their services.

The value $fid comes from user input, so at first glance it would seem you could set it to arbitrary values.

The value comes from the shortcode attribute “form_id”, which comes from user input in the situation Sucuri describes:

9
10
11
12
13
$arrInfo = 	shortcode_atts(
				array( 'form_id' => '', 'show' => '', 'hide' => '', 'display' => '', 'search' => '', 'id' => '', 'class' => '', 
					'header' => '', 'style' => '', 'max_entries' => '', 'start-date' => '', 'end-date' => '' ),
				$atts
			);

Which becomes the value of $formIds:

14
$formIds = $arrInfo['form_id'];

In then becomes $formArr:

30
$formArr = explode(",", $formIds);

Finally it becomes $fid:

48
foreach($formArr as $fid){

None of that code or the other code that runs changes the original value in a way that impacts the user input’s ability to lead to SQL injection.

What we found when starting testing this out before adding the claimed vulnerability to our service’s data set, is that what stops it from being set to an arbitrary value and then used in a SQL statement is this:

56
57
58
$form = vsz_cf7_get_the_form_list($fid);
 
if(!empty($form)){

That will set the value of the variable $form to the result of the function vsz_cf7_get_the_form_list() and then if that isn’t empty further code runs that gets you to the SQL statements shown before. For the value returned from that function to not be empty then $fid has to be an integer since it has to match the ID value of a Contact Form 7 form:

156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
function vsz_cf7_get_the_form_list($fid = ''){
 
	//Get All form information
	$forms = WPCF7_ContactForm::find();
	$form = array();
	//fetch each form information
 
	foreach ($forms as $k => $v){
		//Check if form id not empty then get specific form related information
		if(!empty($fid)){
			if($v->id() == $fid){
				$form[] = $v;
				return $form;
			}
		}
		else{
			$form[] = $v;
		}
    }
 
	if(count($form)>1){
		// New function Added to sort the array by CF7 Name
		usort($form, "cmp_sort_form_name");
	}
 
	return $form;
}

At best you could describe this as a possible or potential vulnerability, but Sucuri didn’t present it that way.

If this wasn’t intentional, it is a good reminder why not including a proof of concept for claimed vulnerabilities can have a negative impact.

WPScan Vulnerability Database and Astra Didn’t Check Things Either

While we actually review things like this before putting them in our data set, other data sources don’t. Here is the WPScan Vulnerability Database’s entry for this:

In looking into we also found that another web security company named Astra largely copy Sucuri’s post without credit (the lack credit probably has to do with simply copying information Sucuri already provided, which Sucuri also does) and make similar false claims, including this:

The risks attached to this vulnerability can be put into the critical category for it could be further exploited by ill intenders. This vulnerability could also act as free entry for hackers to insert dirty codes into the database and get access to valuable data.

In a nutshell, these could go wrong:

  • Bad actors could insert malicious content in the database
  • Hackers can leak sensitive data
  • This could also lead to a compromised WordPress installation.

Astra’s post, like Sucuri’s, seems to be in part about advertising a service and based on them failing to notice that there wasn’t the issue they claimed, it probably wouldn’t be surprising if this service doesn’t work well:

To protect your website from many such possible attacks you can use Astra’s Malware Scanner which comes for just $19/month. Also, with our VAPT (Vulnerability Assessment and Penetration Testing) offering, our engineers will check your website thoroughly and mend all the possible vulnerabilities.

Leave a Reply

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