diff --git a/cleantalk.php b/cleantalk.php index 8ae8e2685..0e5b58dc8 100644 --- a/cleantalk.php +++ b/cleantalk.php @@ -2633,7 +2633,7 @@ function apbct_cookie() // Cookie names to validate $cookie_test_value = array( 'cookies_names' => array(), - 'check_value' => $apbct->api_key, + 'check_value' => $apbct->api_key . $apbct->data['salt'], ); // We need to skip the domain attribute for prevent including the dot to the cookie's domain on the client. @@ -2728,11 +2728,11 @@ function apbct_cookies_test() return 0; } - $check_string = $apbct->api_key; + $check_string = $apbct->api_key . $apbct->data['salt']; // generate value $cookie_names = TT::getArrayValueAsArray($cookie_test, 'cookies_names'); foreach ( $cookie_names as $cookie_name ) { - $check_string .= Cookie::get($cookie_name); + $check_string .= TT::toString(Cookie::get($cookie_name)); } // check generated value with current cookie $check_value = TT::getArrayValueAsString($cookie_test, 'check_value'); diff --git a/tests/RootFile/TestCheckValueSalt.php b/tests/RootFile/TestCheckValueSalt.php new file mode 100644 index 000000000..e64d1be33 --- /dev/null +++ b/tests/RootFile/TestCheckValueSalt.php @@ -0,0 +1,148 @@ +apbctBackup = $apbct; + $this->cookieBackup = $_COOKIE; + + $apbct = (object) [ + 'api_key' => 'testkey123', + 'data' => [ + 'salt' => 'test_salt_value', + 'cookies_type' => 'native', + 'key_is_ok' => 1, + ], + ]; + + \Cleantalk\ApbctWP\Variables\Cookie::$force_alt_cookies_global = false; + } + + protected function tearDown(): void + { + global $apbct; + $apbct = $this->apbctBackup; + $_COOKIE = $this->cookieBackup; + } + + /** + * Test that check_value with salt differs from check_value without salt. + * This verifies the vulnerability fix: check_value must NOT equal md5(api_key). + */ + public function testCheckValueWithSaltDiffersFromWithout() + { + global $apbct; + + $with_salt = md5($apbct->api_key . $apbct->data['salt']); + $without_salt = md5($apbct->api_key); + + $this->assertNotEquals( + $without_salt, + $with_salt, + 'check_value must not equal md5(api_key) alone - salt must change the hash' + ); + } + + /** + * Test that apbct_cookies_test() validates a cookie computed WITH salt. + */ + public function testApbctCookiesTestValidatesWithSalt() + { + global $apbct; + + $cookie_test_value = array( + 'cookies_names' => array(), + 'check_value' => md5($apbct->api_key . $apbct->data['salt']), + ); + + $cookie_prefix = function_exists('apbct__get_cookie_prefix') ? apbct__get_cookie_prefix() : ''; + $_COOKIE[$cookie_prefix . 'apbct_cookies_test'] = urlencode(json_encode($cookie_test_value)); + + $result = apbct_cookies_test(); + + $this->assertSame(1, $result, 'Cookie with md5(api_key + salt) must pass validation'); + } + + /** + * Test that apbct_cookies_test() rejects a cookie computed WITHOUT salt. + * This is the core of the vulnerability fix verification. + */ + public function testApbctCookiesTestRejectsCookieWithoutSalt() + { + global $apbct; + + $cookie_test_value = array( + 'cookies_names' => array(), + 'check_value' => md5($apbct->api_key), // No salt - old vulnerable value + ); + + $cookie_prefix = function_exists('apbct__get_cookie_prefix') ? apbct__get_cookie_prefix() : ''; + $_COOKIE[$cookie_prefix . 'apbct_cookies_test'] = urlencode(json_encode($cookie_test_value)); + + $result = apbct_cookies_test(); + + $this->assertSame(0, $result, 'Cookie with md5(api_key) without salt must be rejected'); + } + + /** + * Test that check_value with salt + timestamp is correctly validated. + */ + public function testApbctCookiesTestValidatesWithSaltAndTimestamp() + { + global $apbct; + + $timestamp = '1718000000'; + + $cookie_test_value = array( + 'cookies_names' => array('ct_ps_timestamp'), + 'check_value' => md5($apbct->api_key . $apbct->data['salt'] . $timestamp), + ); + + $cookie_prefix = function_exists('apbct__get_cookie_prefix') ? apbct__get_cookie_prefix() : ''; + $_COOKIE[$cookie_prefix . 'apbct_cookies_test'] = urlencode(json_encode($cookie_test_value)); + $_COOKIE[$cookie_prefix . 'ct_ps_timestamp'] = $timestamp; + + $result = apbct_cookies_test(); + + $this->assertSame(1, $result, 'Cookie with md5(api_key + salt + timestamp) must pass validation'); + } + + /** + * Test that check_value with timestamp but WITHOUT salt is rejected. + */ + public function testApbctCookiesTestRejectsTimestampCookieWithoutSalt() + { + global $apbct; + + $timestamp = '1718000000'; + + $cookie_test_value = array( + 'cookies_names' => array('ct_ps_timestamp'), + 'check_value' => md5($apbct->api_key . $timestamp), // No salt + ); + + $cookie_prefix = function_exists('apbct__get_cookie_prefix') ? apbct__get_cookie_prefix() : ''; + $_COOKIE[$cookie_prefix . 'apbct_cookies_test'] = urlencode(json_encode($cookie_test_value)); + $_COOKIE[$cookie_prefix . 'ct_ps_timestamp'] = $timestamp; + + $result = apbct_cookies_test(); + + $this->assertSame(0, $result, 'Cookie with md5(api_key + timestamp) without salt must be rejected'); + } +}