Exploiting vulnerability in logical operators “isset (…) && !Anything”

First of all, lets have a look at this code:

    if ( isset( $_POST['crp_export_settings_nonce'] ) && ! wp_verify_nonce( sanitize_key( $_POST['crp_export_settings_nonce'] ), 'crp_export_settings_nonce' ) ) {
        return;
}

This code was taken from Contextual Related, a WordPress plugin with 80,000 active downloads.

Original post: https://security.devsoftin.com/blog/2020/11/19/contextual-related-posts-2-9-4-csrf-nonce-validation-bypass/

This condition is a security validation to block CSRF(Cross-site request forgery), it is used to authenticate the user.

CSRF

In a CSRF attack, an innocent end user is tricked by an attacker into submitting a web request that they did not intend. This may cause actions to be performed on the website that can include inadvertent client or server data leakage, change of session state, or manipulation of an end user’s account.

-Wikipedia

The function wp_verify_nonce checks if the user really came from the previous page and not from an outside request.

But that validation doesn’t work as intended and can be easily bypassed.

This not only is a serious problem, but with other vulnerabilities can be fatal.

In the following video, we can see a simple code scan of a plugin list to check for this issue, and there were a total of 420 plugins with possibly the same issue.

Now let’s analyze the code ourselves:

First things first, everyone needs to be familiar with the logical operators:

For more info: https://www.php.net/manual/en/language.operators.logical.php and https://www.php.net/manual/en/language.operators.precedence.php

Other examples:

Example 1: if( isset($_GET['test']) && strpos($_GET['test'],'a') === false ) die();

Send the ‘test’ parameter as null and you will bypass the isset validation.

Example 2: if( ! isset($_GET['test']) && strpos($_GET['test'],'a') === false ) die();

Send the ‘test’ parameter with an unexpected value like ‘b’ and you bypassed the strpos validation.

Example 3: if( ! empty($_GET['test']) && strpos($_GET['test'],'a') === false ) die();

As in the first example, just send ‘test’ as null and you can bypass the isset validation.

But why does this happen and what happens in the developers’ minds?

  • _GET, _POST and _REQUEST are arrays. When called, they need to go through an index check or always generate an undefined error if the key that we are searching by doesn’t exist. That basically means: it needs an isset or empty.
  • In the case of a condition that wasn’t met, we need to stop and don’t give access to the user.
  • The next validations should be done with OR(||) statements. This so every single condition can be validated, without skips.
  • Usually developers think the other way around, verify that the value exists first AND if it does, check the value. But as we see, that causes the issue we are dealing with. Because we need to throw the error if the value doesn’t exist too!

Here we have a solution to the issue:

if(! isset($_GET['test']) || strpos($_GET['test'],'a') === false ) die();

Some tips for this not to happen:

  • Pay attention when you want to use return early in a method, it can make the conditions harder to write and the quality you are trying to provide become a vulnerability.
  • If in doubt using an early return, for safety, create an if for each condition before simplifying them.
  • Check all the indexes of _POST, _GET, _REQUEST before. If not, preset the variable with the value false. There are frameworks that already do it.
  • Test all the possible ways, sending all parameters, not sending parameters, random values, null values, etc.

References:

https://lenonleite.com.br/en/2020/09/19/explorando-vulnerabilidade-em-operadores-logicos-isset-algumacoisa/

https://blog.nintechnet.com/25-wordpress-plugins-vulnerable-to-csrf-attacks/

https://www.php.net/manual/en/index.php

https://wordpress.org/plugins/contextual-related-posts/

https://plugins.trac.wordpress.org/changeset/2387037/contextual-related-posts/trunk/includes/admin/modules/tools.php

Author: André Cabaça

Leave a Reply

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