We often say that a lot of the people in the security industry don’t know and or care much about security, and we unfortunately keep coming across examples of that. The latest example involves a really bad vulnerability assessment that we ran across while looking in to what recently happened with the plugin Display Widgets, which involved the new owner of the plugin placing malicious code in to it. While looking into that situation we noticed that the changelog entry for version 2.6.3 of the plugin said:

Fixed a vulnerability highlighted by one of our users. I encourage all my users to upgrade as soon as possible.

If you looked over the referenced PDF at phpbuilt.com it looks rather professionally done at first glance, but a closer look shows that the information in the report is very incorrect.

Before we get in to the details what is worth mentioning is the information at the top of the report, since some of it seems at odds with what the report shows about the person behind it. The writer of the report describes themselves as a “PHP and WordPress Plugin Developer”.

They also list the following items under the header “PHP”:

Custom Application Development
Database Performance Consultant
Web Scraping and Data Mining
Zend PHP Certification

And the following items under the heading “WordPress”:

Custom Plugin Development
Security Vulnerability Testing
Wordpress Theme Developer
Zend Framework Certification

(That is all copied from the PDF, so the missing capitalization of the “p” in WordPress is not a mistake on our part.)

Here is the summary of the supposed issue:

Summary: A simple description of the vulnerability of the Display Widgets wordpress plugin is below, followed by
a more thorough explanation.
1. A remote file is included via wp_remote_get in geolocation.php ( http://geoip2.io/api/update/?url= ) inside
geolocation.php
2. Part of the URL includes a variable containing the user’s IP address ( &ip= )
3. The IP utilizes a header ( X-Forwarded-For ) which can be spoofed, and is not filtered in the code.
4. WordPress’s method of page retrieval (wp_remote_get) is a CURL wrapper.
5. Curl accepts multiple addresses in one request separated by space (
https://curl.haxx.se/docs/httpscripting.html section 3.3 )
TLDR; problem: It is possible to forge an X-Forwarded-For header, injecting a second URL to be requested by
the client, turning it into a remote file inclusion vulnerability.

The first problem with this isn’t too hard to understand. Remote file inclusion (RFI) involves including a file from a remote location. In PHP there are several functions that come to mind that could be used for that, include(), include_once(), require(), and require_once(). The function include() will caused the specified file to evaluated (or in simpler terms that code in the file to be run). The function require() identical except it will produced a fatal error if the inclusion doesn’t work. The _once variants of those two will cause an included file to only be included once if there a multiple requests to included it. There are other functions that depending on how wide you wide to stretch the term could be utilized as well, including eval().

In the summary it says that the “file is included via wp_remote_get”, but the wp_remote_get() function doesn’t include files at all, it just makes a request for a URL. Nowhere in the report is there any mention of how the URL requested would actually be included and the only use of any inclusion function is in fact to the load the file that the code mentioned was contained in:

include_once( plugin_dir_path( __FILE__ ) . '/geolocation.php' );

If the person doing this report was not in on the malicious code being added to the plugin, they appear to have not had a basic understanding of what is remote file inclusion is and not read the three pages they linked to that explain that type of vulnerability:

Some examples of how remote file inclusion can hack a wordpress installation:
http://www.hackeroyale.com/hacking-websites-using-rfi-remote-file-inclusion-attack/
https://hack2rule.wordpress.com/category/remote-file-inclusion/
http://hackaday.com/2017/07/31/the-dark-arts-remote-file-inclusion/

There is another huge problem with this that doesn’t require someone to have security knowledge, but just general coding knowledge, which the person behind the report certainly seems to be claiming to have.

Here is the relevant code for item 2 of the summary:

&ip=' . urlencode( $_SERVER[ 'REMOTE_ADDR' ] ) .

That contradicts item 3 of the summary, which states that “The IP utilizes a header ( X-Forwarded-For ) which can be spoofed, and is not filtered in the code.”.

The only place “X-Forwarded-For” is utilized is in the function get_remote_ip():

22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42

public static function get_remote_ip() {
	$remote_ip = '';
 
	if ( !empty( $_SERVER[ 'REMOTE_ADDR' ] ) && filter_var( $_SERVER[ 'REMOTE_ADDR' ], FILTER_VALIDATE_IP ) !== false ) {
		$remote_ip = $_SERVER[ 'REMOTE_ADDR' ];
	}
 
	$originating_ip_headers = array( 'X-Forwarded-For', 'HTTP_X_FORWARDED_FOR', 'CF-Connecting-IP', 'HTTP_CLIENT_IP', 'HTTP_X_REAL_IP', 'HTTP_FORWARDED', 'HTTP_X_FORWARDED' );
	foreach ( $originating_ip_headers as $a_header ) {
		if ( !empty( $_SERVER[ $a_header ] ) ) {
			foreach ( explode( ',', $_SERVER[ $a_header ] ) as $a_ip ) {
				if ( filter_var( $a_ip, FILTER_VALIDATE_IP ) !== false && $a_ip != $remote_ip ) {
					$remote_ip = $a_ip;
					break;
				}
			}
		}
	}
 
	return $remote_ip;
}

That clearly is the usage they are referring to based on where they explain to placed their solution for the claimed vulnerability:

This code should be inserted at line #35 (immediately under the line: $remote_ip = $a_ip;) in the file
geolocation.php

The function get_remote_ip() is never used in the plugin, so we don’t understand how they thought that code in it could have been involved in a vulnerability.

Getting a Plugin’s Security Reviewed

Based on our experience dealing with lots of hacked websites, as well as finding vulnerabilities in plugins that look like they are already being exploited, and doing security review of plugins, our recommendation is that if you think that a plugin might have been the source of the hack, you shouldn’t be looking to have a security review of it done. Instead we would recommend hiring someone that properly cleans up hacked website, which involves determining how the website was hacked, to review the situation. Based on past experience there is a good chance the plugin was not the cause of the hacking. If it was, someone that knows what they are doing when it comes to determining how websites are hacked should be able to handle that best.

If you just have a general concern about the security of a WordPress plugin and you want to hire someone to review of its security what we would tell you to be wary, because what we have seen of people offering them hasn’t been reassuring that they knew what they were doing. In one case, we came across a company that was offering to do security review of WordPress plugins when we went to report a vulnerability that existed in their plugin. The vulnerability wasn’t serious, though it was the kind that many in the security industry would like you to believe otherwise, but it was caused by a failure to do basic security. In the case of another company, which we had considered hiring to do reviews of plugins before we started doing them ourselves as part of our service, we found that one of their reports incorrectly stated that a vulnerability they had found had been fixed. While that could happen for more understandable reasons, in that case it seems to be due to a fairly fundamental misunderstanding of security. When we contacted them about the possibility that there was mistake in claiming the vulnerability was fixed, we never got a response, which ended our consideration of them.

Another thing that concerns us is that the other companies we have seen offering them don’t provide any information on what they are reviewing for. That could mean that they will review for anything possible, but more likely they are not reviewing for a lot of things and no one can easily point out the limitations of their reviews. By comparison we test for a specific set of items when doing review. In the case of this report it is claimed that every line of code was reviewed:

I can, however, state that remote file inclusion is a serious vulnerability and after going over every line
of code, I’m convinced this is the vulnerability being experienced.

That clearly didn’t produce good results in this instance.