Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 28 additions & 8 deletions lib/Cleantalk/ApbctWP/RemoteCalls.php
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,31 @@ private static function isAllowedWithoutToken($rc)
return in_array($rc, self::$allowedActionsWithoutToken, true);
}

/**
* Delay to wait before the remote call is passed to the host.
*
* Only the whitelisted actions are allowed to delay, and the delay itself is limited
* by self::MAX_DELAY.
*
* @param string $action Remote call action name, without the "action__" prefix
* @param mixed $delay Delay as it came in the request
*
* @return int|null Seconds to sleep, null when the call must be performed without a delay
* @psalm-return int<0, max>|null
*/
private static function getDelayForAction($action, $delay)
{
if ( ! $delay || ! in_array(strtolower($action), self::$allowedActionsWithDelay, true) ) {
return null;
}

$delay = TT::toInt($delay);
$delay = max($delay, 0);
$delay = min($delay, self::MAX_DELAY);

return $delay;
}

public static function checkWithoutToken()
{
global $apbct;
Expand Down Expand Up @@ -215,18 +240,13 @@ public static function perform()
// Flag to let plugin know that Remote Call is running.
$apbct->rc_running = true;

$raw_action = $action;
$action = 'action__' . $action;

if ( method_exists(__CLASS__, $action) ) {
// Delay before perform action - only for whitelisted actions
$current_action = strtolower(Request::getString('spbc_remote_call_action'));
if (
Request::get('delay') &&
in_array($current_action, self::$allowedActionsWithDelay, true)
) {
$delay = Request::getInt('delay');
$delay = max($delay, 0);
$delay = min($delay, self::MAX_DELAY);
$delay = self::getDelayForAction($raw_action, Request::get('delay'));
if ( ! is_null($delay) ) {
sleep($delay);
$params = $_REQUEST;
unset($params['delay']);
Expand Down
2 changes: 1 addition & 1 deletion lib/Cleantalk/Common/Helper.php
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,7 @@ public static function ipResolve($ip)
* @param $host
* @param bool $out
*
* @return bool
* @return bool|string Resolved IP, the host itself when it is already an IP, or $out on failure
* @psalm-suppress PossiblyUnusedMethod
*/
public static function dnsResolve($host, $out = false)
Expand Down
132 changes: 132 additions & 0 deletions tests/ApbctWP/TestRemoteCalls.php
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,138 @@ public function itOnlyAllowsSfwUpdateWorkerForDelay()
$this->assertEquals(['sfw_update__worker'], $allowedActions);
}

/** @test */
public function itAllowsDelayOnlyForWhitelistedAction()
{
$method = new ReflectionMethod(RemoteCalls::class, 'getDelayForAction');
$method->setAccessible(true);

$this->assertSame(5, $method->invoke(null, 'sfw_update__worker', 5));

$this->assertNull($method->invoke(null, 'sfw_update', 5));
$this->assertNull($method->invoke(null, 'debug', 5));
$this->assertNull($method->invoke(null, 'get_fresh_wpnonce', 5));
$this->assertNull($method->invoke(null, 'post_api_key', 5));
$this->assertNull($method->invoke(null, 'install_plugin', 5));
$this->assertNull($method->invoke(null, '', 5));
}

/** @test */
public function itIgnoresActionCaseWhenCheckingDelayPermission()
{
$method = new ReflectionMethod(RemoteCalls::class, 'getDelayForAction');
$method->setAccessible(true);

$this->assertSame(5, $method->invoke(null, 'SFW_UPDATE__WORKER', 5));
$this->assertSame(5, $method->invoke(null, 'Sfw_Update__Worker', 5));
}

/** @test */
public function itDoesNotAllowDelayForPrefixedActionName()
{
// perform() must pass the raw action name, not the one prefixed with 'action__'
$method = new ReflectionMethod(RemoteCalls::class, 'getDelayForAction');
$method->setAccessible(true);

$this->assertNull($method->invoke(null, 'action__sfw_update__worker', 5));
}

/** @test */
public function itReturnsNullWhenDelayParamIsEmpty()
{
// No delay param — the action must be performed right here, not passed to the host
$method = new ReflectionMethod(RemoteCalls::class, 'getDelayForAction');
$method->setAccessible(true);

$this->assertNull($method->invoke(null, 'sfw_update__worker', null));
$this->assertNull($method->invoke(null, 'sfw_update__worker', false));
$this->assertNull($method->invoke(null, 'sfw_update__worker', ''));
$this->assertNull($method->invoke(null, 'sfw_update__worker', '0'));
$this->assertNull($method->invoke(null, 'sfw_update__worker', 0));
}

/** @test */
public function itKeepsAllowedDelayValueAsIs()
{
$method = new ReflectionMethod(RemoteCalls::class, 'getDelayForAction');
$method->setAccessible(true);

$this->assertSame(1, $method->invoke(null, 'sfw_update__worker', 1));
$this->assertSame(3, $method->invoke(null, 'sfw_update__worker', 3));
$this->assertSame(
RemoteCalls::MAX_DELAY,
$method->invoke(null, 'sfw_update__worker', RemoteCalls::MAX_DELAY)
);
}

/** @test */
public function itCapsDelayAtMaxDelay()
{
$method = new ReflectionMethod(RemoteCalls::class, 'getDelayForAction');
$method->setAccessible(true);

$this->assertSame(
RemoteCalls::MAX_DELAY,
$method->invoke(null, 'sfw_update__worker', RemoteCalls::MAX_DELAY + 1)
);
$this->assertSame(RemoteCalls::MAX_DELAY, $method->invoke(null, 'sfw_update__worker', 999));
$this->assertSame(RemoteCalls::MAX_DELAY, $method->invoke(null, 'sfw_update__worker', PHP_INT_MAX));
}

/** @test */
public function itTurnsNegativeDelayIntoZeroButStillDelegatesToHost()
{
// Zero, not null: the call is still passed to the host, it just does not sleep
$method = new ReflectionMethod(RemoteCalls::class, 'getDelayForAction');
$method->setAccessible(true);

$this->assertSame(0, $method->invoke(null, 'sfw_update__worker', -1));
$this->assertSame(0, $method->invoke(null, 'sfw_update__worker', -999));
$this->assertSame(0, $method->invoke(null, 'sfw_update__worker', PHP_INT_MIN));
}

/** @test */
public function itCastsNonIntegerDelayToInt()
{
$method = new ReflectionMethod(RemoteCalls::class, 'getDelayForAction');
$method->setAccessible(true);

$this->assertSame(7, $method->invoke(null, 'sfw_update__worker', '7'));
$this->assertSame(7, $method->invoke(null, 'sfw_update__worker', 7.9));
$this->assertSame(0, $method->invoke(null, 'sfw_update__worker', 'abc'));
$this->assertSame(0, $method->invoke(null, 'sfw_update__worker', array(5)));
}

/** @test */
public function performDelegatesDelayDecisionToGetDelayForAction()
{
$method = new ReflectionMethod(RemoteCalls::class, 'perform');
$source = implode('', array_slice(
file($method->getFileName()),
$method->getStartLine() - 1,
$method->getEndLine() - $method->getStartLine() + 1
));

$this->assertStringContainsString(
"self::getDelayForAction(\$raw_action, Request::get('delay'))",
$source,
'perform() must take the delay decision from getDelayForAction()'
);

// The branch is chosen by null, not by the value: a negative delay is still
// passed to the host instead of running the action locally
$this->assertStringContainsString(
'if ( ! is_null($delay) ) {',
$source,
'perform() must branch on null, not on the delay value'
);

$this->assertStringNotContainsString(
'allowedActionsWithDelay',
$source,
'The whitelist check belongs to getDelayForAction(), not to perform()'
);
}
// =========================================================================
// APBCT-W07: 'api_key' added to $sensitiveData list
// =========================================================================
Expand Down
Loading