Security Concern (ClickATell, ClickSend)

  • Resolved DarkSoroush

    (@darksoroush)


    4 years, 5 months ago

    First, please create a GitHub repository so that people like me can create a pull request or maybe discuss these types of issues more privately. WordPress website is not exactly known for development tools.

    And second, the code created for ClickATell and ClickSend providers is pretty much vulnerable to brute force attacks; not on the login process itself, but rather on the hash of the OTP code sent; which is worse since it is not possible to mitigate.
    Both these plugins send the hash of the phone number and the generated OTP code to the client and since the phone number is known, the attacker only needs to crack the hash for the 9999 possible OTP codes. It takes less than a second for anyone sophisticated enough to crack the hash and to be honest you don’t need to be an expert to do so. So its a quite serious problem that might allow access to an administrator account to anyone with little knowledge.

    Following is a quick fix for this; just replace
    $hash = md5( $str_mobile . $str_otp );
    with
    $hash = md5( md5( $str_mobile . $str_otp ) . wp_salt());
    in “class-olws-clickatell-api.php” and “class-olws-clicksend-api.php” files.

    This simple change makes it a lot harder for anyone to crack the hash. A better solution would be to keep the hash in a table and not send it to the user. Or at least keep it in the PHP session.

    https://www.php.net/manual/en/reserved.variables.session.php

Viewing 7 replies - 1 through 7 (of 7 total)
  • Moderator Jan Dembowski

    (@jdembowski)

    Forum Moderator and Brute Squad

    @darksoroush This isn’t really a topic that needs reporting.

    Thread Starter DarkSoroush

    (@darksoroush)

    Sorry @jdembowski, I thought that the button reports this topic to the plugin developer and hopped to gain attention by a notification or an email. I didn’t expect it to reach the WordPress moderators.

    DarkSoroush thank you for taking time and alerting us
    any security issue worth reporting i don’t understand as to why moderator @darksoroush thinks security concert have special places to report!!

    • This reply was modified 4 years, 5 months ago by livedub.
    Plugin Contributor Smit Patadiya

    (@smitpatadiya)

    Thank you for sharing the details and the solution. We will take a closer look and take the necessary actions.

    Thread Starter DarkSoroush

    (@darksoroush)

    This code is also vulnerable to reply attack. In other words, having a combination of [hash-pin] allows a malicious actor to log in multiple times indefinitely. A way to lessen the effect of this is to define a rolling window that passively expires the hash code. Check the following:

    To generate hash:

    
private function olws_rolling_window($offset = 0) {
	$window_size = 60 * 5;
	return ((int) floor(time() / $window_size)) + $offset;
}

private function olws_generate_otp_hash( $country_code, $phone, $otp, $window = false) {
	if (!$window) {
		$window = $this->olws_rolling_window();
	}

	$mobile_number = $country_code . $phone;

	$str_mobile = strval( trim( $mobile_number ) );
	$str_otp    = strval( trim( $otp ) );
	$hash       = md5(md5(md5($str_mobile . $str_otp) . $window) . wp_salt());
	return $hash;
}

    To verify hash:

    
if ($ref_id !== '') {
	for ($i = -1; $i <= 1; $i++) {
		$window = $this->olws_rolling_window($i);
		$new_hash = $this->olws_generate_otp_hash($country_code, $phone, $otp, $window);
		if ($new_hash === $ref_id) {
			$success = true;
			break;
		}
	}
}

$invalid_otp = !$success;

    This can and should be improved by at least keeping the last hash as a user meta information so that it can not be used more than once. But that required more changes to the main class and I was reluctant to do so. This plugin, however, must absolutely take care of this.

    • This reply was modified 4 years, 5 months ago by DarkSoroush.

    @darksoroush For the future: please avoid discussing unfixed security stuff in public channels. Since this one was on the lighter side, I did approve it for publication, though.

    Thread Starter DarkSoroush

    (@darksoroush)

    @tobifjellner, Sorry about that, but I am not familiar with the WordPress forums. Is there a way to mark a topic as Security Related so that it is only visible to the author or one like me needs to find another way to contact the plugin developer? Because that wouldn’t be very convenient, to be honest.

Viewing 7 replies - 1 through 7 (of 7 total)
  • The topic ‘Security Concern (ClickATell, ClickSend)’ is closed to new replies.