4 Aug 2021

There Are So Many Issues With Jetpack’s Post on Claim of a “Very Severe” Vulnerability in a WordPress Plugin

Often blog posts from companies offering security services read like an inadvertent warning that these companies are dishonest and lack a basic grasp of security, if you read by someone also in the field. That is the case with a recent post on the blog of Automattic’s Jetpack service, which both overstates the impact of a vulnerability, while also indicating that the author and the rest of their security team don’t have a basic grasp of the security issue involved here. Making that not all too surprising is that the author of the post is a former employee of an incredibly shady security company, Sucuri.

“Very Severe”

One of the problems we have long seen with security companies discussing vulnerabilities in WordPress plugins is that they overstate the impact of them. Jetpack’s post is titled “Severe Vulnerability Patched In WooCommerce Currency Switcher” and in the first sentence they claim that the vulnerability is a “very severe local file inclusion vulnerability”. Would you guess based on that, that the vulnerability is highly unlikely to be exploited and doesn’t have any impact on its own? We would guess not.

Before getting in to that though, this isn’t actually a local file inclusion (LFI) vulnerability, but an authenticated local file inclusion (LFI) vulnerability. That is an important detail since it means that the attack would require someone logged in to WordPress to be involved. Though less than usual, since the plugin works with Automattic’s WooCommerce plugin and that plugin by default allows untrusted individuals access to WordPress accounts. Curiously, the post doesn’t mention that element of this, despite being highly relevant.

While not explaining those things, it does mention this:

This security flaw could enable attackers to leak sensitive information like database credentials, cryptographic keys, and may allow arbitrary code execution in some instances.

All that local file inclusion (LFI) does on its own is cause the code in a file already on the website to run. For the vulnerability to lead to anything like what was mentioned in the quoted paragraph, there would need to be a file with code that otherwise the attacker could not cause to run. So, say, an attacker could upload an image file to the website, they could cause the code to run, while that normally wouldn’t be possible.

It is hard to see how this could be a severe, much less a very severe vulnerability, but what isn’t hard to see is that by talking about the worst case scenario, but being silent on the reality here, seems unprofessional.

Security Theater

In another instance of pushing the “seriousness” of this vulnerability, the post the author writes this:

Due to the seriousness of this vulnerability, and the number of affected sites, we will delay posting the proof of concept to give users time to upgrade.

The reality here is that they walked through how to exploit this, so the proof of concept isn’t necessary to figure out how to exploit it. Either the author of the post has a really bad grasp of security or that is an instance of security theater.

Still Not Properly Secured

Once you get past the overstated impact here, there is another pretty big issue here. The plugin still isn’t properly secured and Jetpack team doesn’t seem to understand that.

In discussing usage of the PHP extract() function, the author of the post writes this:

The render_html method, which is used by the [woocs] shortcode to display the currency switcher on a page, does not correctly handle arguments sent to PHP’s extract function.

That doesn’t really make sense and if you look at the code before and after the fix for the vulnerability, you can see that things are still insecure in a way that shouldn’t be happening.

In reality, the issue wasn’t that a function did “not correctly handle arguments sent to PHP’s extract function”, it is that the extract() function was being used where it shouldn’t.

The documentation for that function has this warning:

WARNING Do not use extract() on untrusted data, like user input (e.g. $_GET, $_FILES).

That is exactly is what was and still is being done in this plugin, both in the relevant code to the vulnerability, but elsewhere.

The relevant code involves a shortcode, woocs, which causing the function woocs_shortcode() to be run:

434
add_shortcode('woocs', array($this, 'woocs_shortcode'));

That function takes user input in the form of shortcode attributes and passes it in to the function render_html():

2162
public function woocs_shortcode($args) {
2183
	return $this->render_html(WOOCS_PATH . 'views/shortcodes/woocs.php', $args);

Then it is being passed through the extract() function, despite that being warned against by the documentation:

3221
3222
3223
3224
3225
public function render_html($pagepath, $data = array()) {
	if (isset($data['pagepath'])) {
		unset($data['pagepath']);
	}
	@extract($data);

That is how the code looks now, not before the vulnerability was fixed.

In addition to that, there are two more instances of this that are easy to spot, as they can be picked up by our Plugin Security Checker:

This plugin is extracting potentially untrusted user input from shortcode attributes. The documentation of PHP warns against doing extraction of untrusted user input because of the security risk involved in that. Its usage is also officially discouraged by the coding standards for WordPress. File: /woocommerce-currency-switcher/classes/woocs.php Code: 2228 extract(shortcode_atts(array( File: /woocommerce-currency-switcher/classes/woocs.php Code: 2246 extract(shortcode_atts(array('value' => 0), $atts));

If one person missed that, it is somewhat concerning, but apparently the whole Jetpack Scan team missed this:

Thanks to the rest of the Jetpack Scan team for feedback, help, and corrections. Also, thanks to PluginUs.net for swiftly addressing this issue and releasing the updated version.

WordPress Security Needs Improvements

When members of Automattic’s security team appear to be unaware of basic security, it would appear to impact the larger WordPress ecosystem, as on WordPress’ Security page there is this claim about the security team for WordPress:

The WordPress Security Team is made up of approximately 50 experts including lead developers and security researchers — about half are employees of Automattic (makers of WordPress.com, the earliest and largest WordPress hosting platform on the web), and a number work in the web security field. The team consults with well-known and trusted security researchers and hosting companies.

Almost no information is provided on the security team, for example, there isn’t a link on that page to a roster, so it is hard to verify that claim, but assuming it is true, that seems to be a concern.

This situation is a good example of where things could easily be improved with the security of WordPress plugins in the WordPress Plugin Directory and isn’t, since it would be easy to flag instances of this issue that were picked up our Plugin Security Checker and warn developers about that, but that hasn’t happened. Or just warn about in general, considering the WordPress PHP Coding Standards page says to not use it all.

We contacted Automattic over a week ago to suggest that they could help to improve this situation (and to mention the apparent lack of understanding of security by that team); we have yet to hear back from them.

Jetpack’s post raises another potential security issue:

Since WordPress allows any logged-in users to render shortcodes, regardless if they have post-editing privileges or not, this is a pretty severe bug.

That is left hanging there, with no discussion of why that is and if that should be that way, which is odd.

Not Staying Ahead of the Threat

At the end of their post they make this claim:

At Jetpack we work hard to make sure your websites are protected from these types of vulnerabilities. To stay one step ahead of any new threats, check out Jetpack Scan, which includes security scanning and automated malware removal.

If you click the link “websites are protected from these types of vulnerabilities” it doesn’t take you to a page that explains that claim, as what it mentions is unrelated to finding this type of vulnerability. If they were actually interested in helping to protect against these vulnerabilities, then they would be working to reform the WordPress Plugin Directory Team, which they don’t appear to be doing.

If you are removing malware from a website, you are not staying ahead of the threat, since the threat was already exploited. More concerning with that, is that automated malware removal isn’t what you need there, since what is important is to figure out what the threat was, which manually removing the malware is partly about, and fixing that.

Conflict of Interest

The head of Automattic, Matt Mullenweg, is also the head of WordPress, creating a conflict of interest here, since Automattic is selling a security service that is likely going to do better sales wise if the security of WordPress is bad. The conflict of interest is more a problem when there is a clear improvement that could be made regarding usage of extract() and it isn’t happening.


Plugin Security Scorecard Grade for Jetpack

Checked on November 24, 2024
F

See issues causing the plugin to get less than A+ grade

Leave a Reply

Your email address will not be published.