Two days ago, WPScan described a vulnerability fixed in the WordPress plugin Ninja Forms the day before this way:

The plugin does not validate merge tags provided in the request, which could allow unauthenticated attackers to call any static method present in the blog. One from the plugin in particular could allow for PHP Object Injection when a suitable gadget is also present on the blog. Attackers have been exploiting such issue since June 9th, 2022

No sourcing has been provided for the claims, but we don’t have any reason to doubt them at this point.

Today, through third-party data we monitor, we saw what appeared to be a hacker probing for usage of the plugin.

Wordfence wrote a post about this yesterday that didn’t credit WPScan. Claiming to have independently come across it the next day:

On June 16, 2022, the Wordfence Threat Intelligence team noticed a back-ported security update in Ninja Forms, a WordPress plugin with over one million active installations.

What makes that harder to believe is the second part of that paragraph:

As with all security updates in WordPress plugins and themes, our team analyzed the plugin to determine the exploitability and severity of the vulnerability that had been patched.

That claim isn’t possibly true, as among the issues with it, developers frequently don’t disclose that a new version is a security update (or necessarily even know it has been one). Also, that is contradicted by Wordfence failing to protect against fixed vulnerabilities that appear to have been targeted by hackers before they were fixed.

The next part of their post is more problematic:

We uncovered a code injection vulnerability that made it possible for unauthenticated attackers to call a limited number of methods in various Ninja Forms classes, including a method that unserialized user-supplied content, resulting in Object Injection. This could allow attackers to execute arbitrary code or delete arbitrary files on sites where a separate POP chain was present.

The claim about calling a “limited number of methods in various Ninja Forms classes” doesn’t match what we found. The second part of that is also problematic, as so far it looks to us like the possible PHP object injection vector is restricted by code running before the unserialize function.

Setting Merge Tags From URL

The plugin contains code to allow form submissions to include data from the URL of a form in the submission. That involves adding user input to what the plugin refers to as merge tags. That is is handled in the file /includes/MergeTags/Other.php. That starts the function init():

41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63

public function init()
{
	if( is_admin() ) {
		if( ! defined( 'DOING_AJAX' ) || ! DOING_AJAX ) return;
		$url_query = parse_url( wp_get_referer(), PHP_URL_QUERY );
		parse_str( $url_query, $variables );
	} else {
		$variables = $_GET;
	}
 
	if( ! is_array( $variables ) ) return;
 
	foreach( $variables as $key => $value ){
		if ( is_array( $value ) ) {
			$value = wp_kses_post_deep( $value );
			$value = map_deep( $value, 'esc_attr' );
		} else {
			$value = wp_kses_post( $value );
			$value = esc_attr( $value );
		}
		$this->set_merge_tags( $key, $value );
	}
}

And then goes to function set_merge_tags(), which looked like this in the version 3.6.10:

70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87

public function set_merge_tags( $key, $value )
{
	$callback = ( is_numeric( $key ) ) ? 'querystring_' . $key : $key;
 
	$this->merge_tags[ $callback ] = array(
		'id' => $key,
		'tag' => "{querystring:" . $key . "}",
		'callback' => $callback,
		'value' => $value
	);
 
	$this->merge_tags[ $callback . '_deprecated' ] = array(
		'id' => $key,
		'tag' => "{" . $key . "}",
		'callback' => $callback,
		'value' => $value
	);
}

In the new version, the beginning of that was changed to restrict values with “::” in them:

77
78
79
80

public function set_merge_tags( $key, $value )
{
	// Remove static callback potential
	if( false !== strpos( $key, '::' ) ) return;

The significance is explained by another function that was changed.

Remote Code Execution (RCE)

In the file /includes/Abstracts/MergeTags.php the function replace() was changed in version 3.6.11. The change restricts usage of “::” of and “new”:

65
66
67
68
69
70
71
72
73
74

		// Remove static callback potential
		if( is_string( $merge_tag['callback'] ) &&
			false !== strpos( $merge_tag['callback'], '::' ) ) {
				$merge_tag['callback'] = NULL;
		} // Remove class initializtion potential
		elseif( is_array( $merge_tag['callback'] )
				&& is_string( $merge_tag['callback'][0] )
				&& 0 === strpos( trim( $merge_tag['callback'][0] ), 'new' ) ) {
			$merge_tag['callback'] = NULL;
		}

Looking at the previous version of the function shows the reason for that:

47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68

public function replace( $subject )
{
	if( is_array( $subject ) ){
		foreach( $subject as $i => $s ){
			$subject[ $i ] = $this->replace( $s );
		}
		return $subject;
	}
 
	preg_match_all("/{([^}]*)}/", $subject, $matches );
 
	if( empty( $matches[0] ) ) return $subject;
 
	foreach( $this->merge_tags as $merge_tag ){
		if( ! isset( $merge_tag[ 'tag' ] ) || ! in_array( $merge_tag[ 'tag' ], $matches[0] ) ) continue;
 
		if( ! isset($merge_tag[ 'callback' ])) continue;
 
		if ( is_callable( array( $this, $merge_tag[ 'callback' ] ) ) ) {
			$replace = $this->{$merge_tag[ 'callback' ]}();
		} elseif ( is_callable( $merge_tag[ 'callback' ] ) ) {
			$replace = $merge_tag[ 'callback' ]();

That code will call code based on that user input. Wordfence makes this claim about that:

Unfortunately, this functionality had a flaw that made it possible to call various Ninja Form classes that could be used for a wide range of exploits targeting vulnerable WordPress sites.

As the proof of concept below confirms, that isn’t the case. As classes outside of the plugin can be called as well.

is_callable Confusion

There seems to be confusion in discussion about this vulnerability about the role of the function is_callable(). Wordfence may have created part of that confusion with this:

Without providing too many details on the vulnerability, the Merge Tag functionality does an is_callable() check on a supplied Merge Tags. When a callable class and method is supplied as a Merge Tag, the function is called and the code executed.

Patchstack added further confusion to this:

There was one difference though – the function that was validating the user-controlled data used the is_callable() function to determine if the given method is part of the proper scope or not. If this was not the case, the user-controlled data would be executed as a variable function outside of the $this scope which makes it a more dynamic attack vector than deserialize().

Just like the insecure usage of deserialize(), is_callable() should not be used with user input or user-controlled data if this user input controlled data is then used to execute a variable function.

There is no large warning in is_callable()’s documentation, like there is in serialize() but, the closest I found to a warning for developers when using is_callable() is the following under Notes:

This function may trigger autoloading if called with the name of a class.
If an object implements __call(), then this function will return true for any method on that object, even if the method is not defined.

In both instances of the code where is_callable() is used, then the value of the user input is used to call code to be run. Possibly there is confusion between that code running and the is_callable() usage right before that.

NF_Admin_Processes_ImportForm

Wordfence’s post also makes this claim:

We determined that this could lead to a variety of exploit chains due to the various classes and functions that the Ninja Forms plugin contains. One potentially critical exploit chain in particular involves the use of the NF_Admin_Processes_ImportForm class to achieve remote code execution via deserialization, though there would need to be another plugin or theme installed on the site with a usable gadget.

The code we already discussed, as the proof of concept confirms, already provides access to what they are mentioning as having to get through another step. The other big problem with this, is if you test things out or carefully look at the code, you find that the code looks to block this from being possible.

What that relates to is the function startup() in the file /includes/Admin/Processes/ImportForm.php. Here is how that looked in the previous version:

67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91

public function startup()
{
	// If we aren't passed any form content, bail.
	if ( empty ( $_POST[ 'extraData' ][ 'content' ] ) ) {
		$this->add_error( 'empty_content', esc_html__( 'No export provided.', 'ninja-forms' ), 'fatal' );
		$this->batch_complete();
	}
	$extra_content = WPN_Helper::esc_html($_POST[ 'extraData' ][ 'content']);
	$data = explode( ';base64,', $extra_content );
	$data = base64_decode( $data[ 1 ] );
 
	/**
	 * $data could now hold two things, depending on whether this was a 2.9 or 3.0 export.
	 * 
	 * If it's a 3.0 export, the data will be json encoded.
	 * If it's a 2.9 export, the data will be serialized.
	 *
	 * We're first going to try to json_decode. If we don't get an array, we'll unserialize.
	 */
 
	$decoded_data = json_decode( WPN_Helper::json_cleanup( html_entity_decode( $data, ENT_QUOTES ) ), true );
 
	// If we don't have an array, try unserializing
	if ( ! is_array( $decoded_data ) ) {
		$decoded_data = WPN_Helper::maybe_unserialize( $data );

While user input is passed to a function maybe_unserialize(), which could lead to PHP object injection, it first passes through the function json_cleanup(). That function restricts the value coming out in a way that in our testing would prevent PHP object injection from occurring.

The confusion may be related to the new version removing that usage of maybe_unserialize().

Patchstack’s Questionable Claim

Patchstack’s promotes what happened here as a success:

This forced update was performed at the request of the Ninja Forms developer because the patch addresses a critical security bug that could have led to limited code execution. The WordPress.org plugin team reviewed the patch, and forced websites to update the plugin, ensuring all sites with Ninja Forms installed received a timely update, even if they have opted out of auto-updates on the plugin.

 

Reviewing the patch’s diff, I can confirm it addresses a security bug that could have been easily weaponized into existing bot-nets, and a successful attack could compromise a site with a single request.

The action of forcing the update, minimized the time it took to secure websites to just a few days. It shortened the patch window and diminished any malicious party’s window of opportunity to compromise websites using this vulnerability.

What isn’t mentioned is that two days before their post came out, it was claimed that the vulnerability was exploited back at least to June 9. Patchstack originally cited WPScan as their source for the vulnerability in their database, so they knew about that, but their post ignores that. That is possibly explained by an apparent need by some in the WordPress community to promote everything in a positive fashion, even when it isn’t true. That isn’t helping security at all.

Proof of Concept

With our plugin for testing for PHP object injection installed and activated, the following proof of concept will cause the message “PHP object injection has occurred.” be shown.

1. Access a page with one of the plugin’s forms.
2. Add “?php_object_injection::__wakeup” to the end of the URL and access the page again.
3. On a form input, add “{php_object_injection::__wakeup}”.
4. Submit the form.
5. You will see the message “PHP object injection has occurred.” in the response to the resulting AJAX request made.

Plugin Security Scorecard Grade for Ninja Forms

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

Plugin Security Scorecard Grade for Patchstack

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

Plugin Security Scorecard Grade for WPScan

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