Skip to content

Fix. SFW. AC/AF pass cookie value fixed. #917

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
svedge merged 2 commits into from
Oct 27, 2026
Merged

Fix. SFW. AC/AF pass cookie value fixed. #917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e2e2041
Fix. SFW. AC/AF pass cookie value fixed.
Glomberg Oct 26, 2026
b385044
Merge branch 'dev' into Fix-SFW-AC/AF-pass-cookie-value-fixed-VI
svedge Oct 27, 2026
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
30 changes: 30 additions & 0 deletions cleantalk.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -904,6 +904,36 @@ function apbct_sfw__check()
}
}

// Skip the check (legacy way, remove in future)
if ( ! empty(Get::get('access')) ) {
$spbc_settings = get_option('spbc_settings');
$spbc_key = ! empty($spbc_settings['spbc_key']) ? $spbc_settings['spbc_key'] : false;
if ( Get::get('access') === $apbct->api_key || ($spbc_key !== false && Get::get('access') === $spbc_key) ) {
Cookie::set(
'spbc_firewall_pass_key',
md5(Server::get('REMOTE_ADDR') . $spbc_key),
time() + 1200,
'/',
''
);
Cookie::set(
'ct_sfw_pass_key',
SFW::makePassKey(
TT::toString(Server::get('REMOTE_ADDR')),
TT::toString($apbct->api_key),
TT::toString($apbct->data['salt']),
'0'
),
time() + 1200,
'/',
''
);

return;
}
unset($spbc_settings, $spbc_key);
}

// Turn off the SpamFireWall if Remote Call is in progress
if ( $apbct->rc_running || ( ! empty($spbc) && $spbc->rc_running) ) {
return;
Expand Down
76 changes: 63 additions & 13 deletions lib/Cleantalk/ApbctWP/Firewall/SFW.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ public function check()
global $apbct;

$results = array();
$status = 0;

if (empty($apbct->key_is_ok) || empty($apbct->api_key)) {
return $results;
Expand All @@ -143,12 +142,16 @@ public function check()
}

// Skip by cookie
$pass_key = Cookie::getString('ct_sfw_pass_key');
foreach ($this->ip_array as $current_ip) {
if (
TT::toString(Cookie::get('ct_sfw_pass_key'))
&& strpos(TT::toString(Cookie::get('ct_sfw_pass_key')), md5($current_ip . $this->api_key . $apbct->data['salt'])) === 0
) {
if (Cookie::get('ct_sfw_passed')) {
$status = self::getPassKeyStatus(
$pass_key,
TT::toString($current_ip),
TT::toString($this->api_key),
TT::toString($apbct->data['salt'])
);
if ($status !== null) {
if (Cookie::getString('ct_sfw_passed')) {
if ( ! headers_sent()) {
Cookie::set(
'ct_sfw_passed',
Expand Down Expand Up @@ -176,11 +179,7 @@ public function check()
}
}

if (strlen(TT::toString(Cookie::get('ct_sfw_pass_key'))) > 32) {
$status = substr(TT::toString(Cookie::get('ct_sfw_pass_key')), -1);
}

if ($status) {
if ($status === '1') {
$results[] = array(
'ip' => $current_ip,
'is_personal' => false,
Expand Down Expand Up @@ -277,6 +276,47 @@ public function check()
return $results;
}

/**
* Builds the ct_sfw_pass_key cookie value. The status is signed together with the IP.
*
* @param string $ip
* @param string $api_key
* @param string $salt
* @param string $status '1' - whitelisted, '0' - passed
*
* @return string
*/
public static function makePassKey($ip, $api_key, $salt, $status)
{
$status = $status === '1' ? '1' : '0';

return md5($ip . $api_key . $salt . $status) . $status;
}

/**
* Validates the ct_sfw_pass_key cookie value.
*
* @param string $pass_key
* @param string $ip
* @param string $api_key
* @param string $salt
*
* @return string|null '1' or '0' for a valid key, null otherwise
*/
public static function getPassKeyStatus($pass_key, $ip, $api_key, $salt)
{
if (strlen($pass_key) !== 33) {
return null;
}

$status = substr($pass_key, -1);
if ($status !== '0' && $status !== '1') {
return null;
}

return hash_equals(self::makePassKey($ip, $api_key, $salt, $status), $pass_key) ? $status : null;
}

/**
* Add entry to SFW log.
* Writes to database.
Expand Down Expand Up @@ -360,7 +400,12 @@ public function actionsForPassed($result)
global $apbct;
if ($this->data__cookies_type === 'native' && ! headers_sent()) {
$status = $result['status'] === 'PASS_SFW__BY_WHITELIST' ? '1' : '0';
$cookie_val = md5($result['ip'] . $this->api_key . $apbct->data['salt']) . $status;
$cookie_val = self::makePassKey(
TT::toString($result['ip']),
TT::toString($this->api_key),
TT::toString($apbct->data['salt']),
$status
);
Cookie::setNativeCookie(
'ct_sfw_pass_key',
$cookie_val,
Expand Down Expand Up @@ -399,7 +444,12 @@ public function diePage($result)
$net_count = $apbct->stats['sfw']['entries'];

$status = $result['status'] === 'PASS_SFW__BY_WHITELIST' ? '1' : '0';
$cookie_val = md5($result['ip'] . $this->api_key . $apbct->data['salt']) . $status;
$cookie_val = self::makePassKey(
TT::toString($result['ip']),
TT::toString($this->api_key),
TT::toString($apbct->data['salt']),
$status
);

$block_message = sprintf(
esc_html__('SpamFireWall is checking your browser and IP %s for spam bots', 'cleantalk-spam-protect'),
Expand Down
99 changes: 73 additions & 26 deletions tests/ApbctWP/Firewall/TestSFW.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ public function testCookieWithSaltIsAccepted()
$api_key = 'test_api_key';
$salt = 'test_salt_value';

// New-style cookie: md5(ip + api_key + salt)
$new_cookie = md5($ip . $api_key . $salt) . '0';
// New-style cookie: md5(ip + api_key + salt + status) + status
$new_cookie = SFW::makePassKey($ip, $api_key, $salt, '0');
$_COOKIE['ct_sfw_pass_key'] = $new_cookie;
$_COOKIE['ct_sfw_passed'] = '1';

Expand Down Expand Up @@ -164,8 +164,8 @@ public function testCookieWithSaltAndWhitelistStatus()
$api_key = 'test_api_key';
$salt = 'test_salt_value';

// New-style cookie with whitelist status '1'
$new_cookie = md5($ip . $api_key . $salt) . '1';
// Whitelist cookie issued by the plugin, status '1' is signed
$new_cookie = SFW::makePassKey($ip, $api_key, $salt, '1');
$_COOKIE['ct_sfw_pass_key'] = $new_cookie;

$sfw = new SFW(
Expand Down Expand Up @@ -200,7 +200,7 @@ public function testCookieWithWrongSaltIsRejected()
$api_key = 'test_api_key';

// Cookie made with wrong salt
$wrong_salt_cookie = md5($ip . $api_key . 'wrong_salt') . '0';
$wrong_salt_cookie = SFW::makePassKey($ip, $api_key, 'wrong_salt', '0');
$_COOKIE['ct_sfw_pass_key'] = $wrong_salt_cookie;

$sfw = new SFW(
Expand Down Expand Up @@ -228,15 +228,72 @@ public function testCookieWithWrongSaltIsRejected()

/**
* @test
* actionsForPassed() sets cookie value using salt.
* makePassKey() signs the status: keys for '0' and '1' differ in the hash part, not only the suffix.
*/
public function testActionsForPassedUsesSaltInCookie()
public function testMakePassKeySignsStatus()
{
global $apbct;
$ip = '203.0.113.50';
$api_key = 'test_api_key';
$salt = 'test_salt_value';

$passed = SFW::makePassKey($ip, $api_key, $salt, '0');
$whitelisted = SFW::makePassKey($ip, $api_key, $salt, '1');

$this->assertSame(md5($ip . $api_key . $salt . '0') . '0', $passed);
$this->assertSame(md5($ip . $api_key . $salt . '1') . '1', $whitelisted);
$this->assertSame(33, strlen($passed));
$this->assertNotSame(substr($passed, 0, 32), substr($whitelisted, 0, 32));
$this->assertSame($passed, SFW::makePassKey($ip, $api_key, $salt, 'x'), 'Unknown status falls back to 0');
}

/**
* @test
* getPassKeyStatus() accepts only exact, correctly signed 33-char keys with status 0 or 1.
*
* @dataProvider passKeyProvider
*/
public function testGetPassKeyStatus($pass_key, $expected)
{
$this->assertSame(
$expected,
SFW::getPassKeyStatus($pass_key, '203.0.113.50', 'test_api_key', 'test_salt_value')
);
}

public function passKeyProvider()
{
$ip = '203.0.113.50';
$api_key = 'test_api_key';
$salt = 'test_salt_value';
$passed = SFW::makePassKey($ip, $api_key, $salt, '0');
$whitelisted = SFW::makePassKey($ip, $api_key, $salt, '1');

return array(
'valid passed' => array($passed, '0'),
'valid whitelisted' => array($whitelisted, '1'),
'status flipped 0 to 1' => array(substr($passed, 0, 32) . '1', null),
'status flipped 1 to 0' => array(substr($whitelisted, 0, 32) . '0', null),
'old unsigned status format' => array(md5($ip . $api_key . $salt) . '1', null),
'status other than 0 or 1' => array(substr($passed, 0, 32) . 'a', null),
'hash only, no status' => array(substr($passed, 0, 32), null),
'extra trailing chars' => array($passed . '1', null),
'other IP' => array(SFW::makePassKey('198.51.100.1', $api_key, $salt, '1'), null),
'empty' => array('', null),
);
}

/**
* @test
* A passed (status 0) cookie with its last char flipped to '1' must not give PASS_SFW__BY_WHITELIST.
*/
public function testForgedWhitelistStatusIsRejected()
{
$ip = '10.0.0.1';
$api_key = 'test_api_key';
$salt = 'test_salt_value';

$forged = substr(SFW::makePassKey($ip, $api_key, $salt, '0'), 0, 32) . '1';
$_COOKIE['ct_sfw_pass_key'] = $forged;

$sfw = new SFW(
'test_log_table',
Expand All @@ -247,26 +304,16 @@ public function testActionsForPassedUsesSaltInCookie()
'sfw_common_table_name' => 'test_data_table',
)
);
$sfw->setIpArray(array('real' => $ip));
$sfw->setDb($this->createDbStub(array()));

$result = array(
'ip' => $ip,
'status' => 'PASS_SFW',
);

// actionsForPassed sets a cookie - we can't easily verify the cookie value
// in CLI, but we can verify the hash computation is correct
$expected_hash = md5($ip . $api_key . $salt) . '0';
$results = $sfw->check();

// Verify that the hash formula includes salt
$this->assertNotEquals(
md5($ip . $api_key) . '0',
$expected_hash,
'Hash with salt must differ from hash without salt'
);
$this->assertEquals(
md5($ip . $api_key . $salt) . '0',
$expected_hash
);
$this->assertNotEmpty($results, 'Forged cookie must fall through to the regular DB check');
foreach ($results as $result) {
$this->assertNotEquals('PASS_SFW__BY_WHITELIST', $result['status']);
$this->assertNotEquals('PASS_SFW__BY_COOKIE', $result['status']);
}
}

// =========================================================================
Expand Down
Loading