diff --git a/common/framework/Security.php b/common/framework/Security.php index 200cf24d2..d5e0802cd 100644 --- a/common/framework/Security.php +++ b/common/framework/Security.php @@ -314,39 +314,57 @@ class Security */ public static function checkCSRF(?string $referer = null): bool { + // If CSRF token checking is enabled, check the token header or parameter first. + // Tokens are allowed to be missing for unauthenticated users. $check_csrf_token = config('security.check_csrf_token') ? true : false; - if ($token = isset($_SERVER['HTTP_X_CSRF_TOKEN']) ? $_SERVER['HTTP_X_CSRF_TOKEN'] : null) + if ($check_csrf_token) { - return Session::verifyToken((string)$token, '', $check_csrf_token); + $token = $_SERVER['HTTP_X_CSRF_TOKEN'] ?? ($_REQUEST['_rx_csrf_token'] ?? null); + if ($token) + { + return Session::verifyToken((string)$token, '', $check_csrf_token); + } + elseif (Session::getMemberSrl()) + { + trigger_error('CSRF token missing in authenticated POST request: ' . (\Context::get('act') ?: '(no act)'), \E_USER_WARNING); + return false; + } } - elseif ($token = isset($_REQUEST['_rx_csrf_token']) ? $_REQUEST['_rx_csrf_token'] : null) - { - return Session::verifyToken((string)$token, '', $check_csrf_token); - } - elseif ($_SERVER['REQUEST_METHOD'] === 'GET') + + // Return early for GET and other "safe" requests. + if (in_array($_SERVER['REQUEST_METHOD'], ['GET', 'HEAD', 'OPTIONS', 'TRACE'])) { return false; } + + // The Sec-Fetch-Site header is the standard way to check whether the request is same-origin. + $sec_fetch_site = $_SERVER['HTTP_SEC_FETCH_SITE'] ?? ''; + if ($sec_fetch_site === 'same-origin' || $sec_fetch_site === 'none') + { + return true; + } + if ($sec_fetch_site === 'cross-site') + { + return false; + } + + // If the Sec-Fetch-Site header is not available, check the Origin header. + $origin = strval($_SERVER['HTTP_ORIGIN'] ?? ''); + if ($origin !== '' && $origin !== 'null') + { + return URL::isInternalURL($origin); + } + + // If the Origin header is not available, fall back to the Referer header. + // The Referer header is NOT allowed to be empty in this case. + $referer = $referer ?? strval($_SERVER['HTTP_REFERER'] ?? ''); + if ($referer !== '' && $referer !== 'null') + { + return URL::isInternalURL($referer); + } else { - $is_logged = Session::getMemberSrl(); - if ($is_logged) - { - trigger_error('CSRF token missing in POST request: ' . (\Context::get('act') ?: '(no act)'), \E_USER_WARNING); - } - - if (!$referer) - { - $referer = strval(($_SERVER['HTTP_ORIGIN'] ?? '') ?: ($_SERVER['HTTP_REFERER'] ?? '')); - } - if ($referer !== '' && $referer !== 'null' && (!$check_csrf_token || !$is_logged)) - { - return URL::isInternalURL($referer); - } - else - { - return false; - } + return false; } } diff --git a/modules/admin/lang/en.php b/modules/admin/lang/en.php index e5081ebce..ff257d238 100644 --- a/modules/admin/lang/en.php +++ b/modules/admin/lang/en.php @@ -187,7 +187,7 @@ $lang->about_use_session_ssl = 'Force the session to be SSL-only.
This helps $lang->use_cookies_ssl = 'Use SSL-only cookies'; $lang->about_use_cookies_ssl = 'Force all cookies to be SSL-only.'; $lang->check_csrf_token = 'Use CSRF tokens'; -$lang->about_check_csrf_token = 'Use CSRF tokens to validate requests. This is more secure but may break some functionality.
If not selected, Rhymix will use only the Referer header to defend against CSRF attacks.'; +$lang->about_check_csrf_token = 'Use CSRF tokens to validate requests. This may break some functionality.
If not selected, Rhymix will use headers such as Sec-Fetch-Site and Origin to block CSRF attacks.'; $lang->use_nofollow = 'Add nofollow attribute to Links'; $lang->about_use_nofollow = 'Add rel="nofollow" to all links submitted by users in order to reduce the effectiveness of spamming.
This does not apply to content submitted by the administrator.'; $lang->use_object_cache = 'Use Cache'; diff --git a/modules/admin/lang/ko.php b/modules/admin/lang/ko.php index a0af5f684..d9a29fbda 100644 --- a/modules/admin/lang/ko.php +++ b/modules/admin/lang/ko.php @@ -188,7 +188,7 @@ $lang->about_use_session_ssl = '세션을 SSL 전용으로 지정하여 SSL이 $lang->use_cookies_ssl = 'SSL 전용 쿠키 사용'; $lang->about_use_cookies_ssl = '세션뿐 아니라 모든 쿠키를 SSL 전용으로 지정합니다.
SSL을 항상 사용하도록 설정되어 있는 경우에만 활성화됩니다.'; $lang->check_csrf_token = 'CSRF 토큰 사용'; -$lang->about_check_csrf_token = 'CSRF 토큰을 사용하여 정상적인 요청 여부를 확인합니다. 보안이 향상되지만, 일부 서드파티 자료와 호환되지 않을 수 있습니다.
사용하지 않을 경우 리퍼러 헤더 확인만으로 CSRF 공격에 방어합니다.'; +$lang->about_check_csrf_token = 'CSRF 토큰을 사용하여 정상적인 요청 여부를 확인합니다. 일부 자료와 호환되지 않을 수 있습니다.
사용하지 않을 경우 Sec-Fetch-Site, Origin 등의 헤더를 사용하여 CSRF 공격을 감지합니다.'; $lang->use_nofollow = '링크에 nofollow 추가'; $lang->about_use_nofollow = '사용자들이 작성한 글에 포함된 모든 링크에 rel="nofollow" 속성을 추가하여 스팸으로 인한 사이트 신뢰도 저하를 방지합니다.
관리자가 작성한 글에는 적용되지 않습니다.'; $lang->use_object_cache = '캐시 사용'; diff --git a/tests/unit/framework/SecurityTest.php b/tests/unit/framework/SecurityTest.php index f637ceb83..4246316be 100644 --- a/tests/unit/framework/SecurityTest.php +++ b/tests/unit/framework/SecurityTest.php @@ -106,10 +106,13 @@ class SecurityTest extends \Codeception\Test\Unit { $error_reporting = error_reporting(0); + config('security.check_csrf_token', true); + $_SERVER['REQUEST_METHOD'] = 'GET'; $_SERVER['HTTP_REFERER'] = ''; $_SERVER['HTTP_X_CSRF_TOKEN'] = ''; $this->assertFalse(Rhymix\Framework\Security::checkCSRF()); + $_SERVER['HTTP_X_CSRF_TOKEN'] = Rhymix\Framework\Session::createToken(); $this->assertTrue(Rhymix\Framework\Security::checkCSRF()); @@ -117,6 +120,7 @@ class SecurityTest extends \Codeception\Test\Unit $_SERVER['HTTP_REFERER'] = ''; $_SERVER['HTTP_X_CSRF_TOKEN'] = ''; $this->assertFalse(Rhymix\Framework\Security::checkCSRF()); + $_SERVER['HTTP_X_CSRF_TOKEN'] = Rhymix\Framework\Session::createToken(); $this->assertTrue(Rhymix\Framework\Security::checkCSRF()); @@ -124,27 +128,63 @@ class SecurityTest extends \Codeception\Test\Unit $_SERVER['HTTP_X_CSRF_TOKEN'] = ''; $this->assertFalse(Rhymix\Framework\Security::checkCSRF()); - $_SERVER['HTTP_REFERER'] = 'http://www.rhymix.org/foo/bar'; - $_SERVER['HTTP_X_CSRF_TOKEN'] = ''; + $_SERVER['HTTP_X_CSRF_TOKEN'] = Rhymix\Framework\Session::createToken(); $this->assertTrue(Rhymix\Framework\Security::checkCSRF()); + $_SERVER['HTTP_X_CSRF_TOKEN'] = 'invalid value'; $this->assertFalse(Rhymix\Framework\Security::checkCSRF()); - $_SERVER['HTTP_ORIGIN'] = 'http://www.rhymix.org'; - $_SERVER['HTTP_REFERER'] = 'http://www.foobar.com'; - $_SERVER['HTTP_X_CSRF_TOKEN'] = ''; - $this->assertTrue(Rhymix\Framework\Security::checkCSRF()); - $_SERVER['HTTP_REFERER'] = ''; - $this->assertTrue(Rhymix\Framework\Security::checkCSRF()); - $_SERVER['HTTP_ORIGIN'] = 'http://www.foobar.com'; + $_SERVER['HTTP_X_CSRF_TOKEN'] = '0'; $this->assertFalse(Rhymix\Framework\Security::checkCSRF()); + + config('security.check_csrf_token', false); + unset($_SERVER['HTTP_X_CSRF_TOKEN']); + + $_SERVER['HTTP_REFERER'] = 'https://www.rhymix.org/foo/bar'; + $this->assertTrue(Rhymix\Framework\Security::checkCSRF()); + + $_SERVER['HTTP_ORIGIN'] = 'https://www.rhymix.org'; + $_SERVER['HTTP_REFERER'] = 'https://www.foobar.com'; + $this->assertTrue(Rhymix\Framework\Security::checkCSRF()); + + $_SERVER['HTTP_ORIGIN'] = 'https://www.foobar.com'; + $this->assertFalse(Rhymix\Framework\Security::checkCSRF()); + + $_SERVER['HTTP_SEC_FETCH_SITE'] = 'same-origin'; + $this->assertTrue(Rhymix\Framework\Security::checkCSRF()); + + $_SERVER['HTTP_SEC_FETCH_SITE'] = 'none'; + $this->assertTrue(Rhymix\Framework\Security::checkCSRF()); + + $_SERVER['HTTP_SEC_FETCH_SITE'] = 'invalid value'; + $this->assertFalse(Rhymix\Framework\Security::checkCSRF()); + + unset($_SERVER['HTTP_SEC_FETCH_SITE']); + + $_SERVER['HTTP_ORIGIN'] = ''; + $_SERVER['HTTP_REFERER'] = ''; + $this->assertFalse(Rhymix\Framework\Security::checkCSRF()); + $_SERVER['HTTP_ORIGIN'] = 'null'; + $_SERVER['HTTP_REFERER'] = ''; $this->assertFalse(Rhymix\Framework\Security::checkCSRF()); - $_SERVER['HTTP_REFERER'] = ''; - $_SERVER['HTTP_X_CSRF_TOKEN'] = ''; - $this->assertTrue(Rhymix\Framework\Security::checkCSRF('http://www.rhymix.org/')); + $_SERVER['HTTP_ORIGIN'] = 'null'; + $_SERVER['HTTP_REFERER'] = 'https://www.rhymix.org'; + $this->assertTrue(Rhymix\Framework\Security::checkCSRF()); + $_SERVER['HTTP_ORIGIN'] = ''; + $_SERVER['HTTP_REFERER'] = 'null'; + $this->assertFalse(Rhymix\Framework\Security::checkCSRF()); + + $_SERVER['HTTP_ORIGIN'] = ''; + $_SERVER['HTTP_REFERER'] = ''; + $this->assertTrue(Rhymix\Framework\Security::checkCSRF('https://www.rhymix.org/')); + + $_SERVER['HTTP_SEC_FETCH_SITE'] = 'cross-site'; + $this->assertFalse(Rhymix\Framework\Security::checkCSRF('https://www.rhymix.org/')); + + unset($_SERVER['HTTP_SEC_FETCH_SITE']); error_reporting($error_reporting); }