diff --git a/htdocs/modules/system/admin.php b/htdocs/modules/system/admin.php
index 295ff18ca..d5b551ead 100644
--- a/htdocs/modules/system/admin.php
+++ b/htdocs/modules/system/admin.php
@@ -81,6 +81,7 @@
$op = Request::getString('op', '');
if ($op === 'system_activate') {
\Xoops::getInstance()->logger()->quiet();
+ $xoops->security()->requireAjaxToken();
$part = Request::getString('type', '');
$config_handler = $xoops->getHandlerConfig();
@@ -102,6 +103,7 @@
// Define scripts
$xoops->theme()->addBaseScriptAssets('@jquery');
$xoops->theme()->addBaseScriptAssets('modules/system/js/admin.js');
+ system_addAjaxToken();
// Define Breadcrumb and tips
$admin_page = new \Xoops\Module\Admin();
$admin_page->addBreadcrumbLink(SystemLocale::CONTROL_PANEL, \XoopsBaseConfig::get('url') . '/admin.php', true);
diff --git a/htdocs/modules/system/admin/blocksadmin/main.php b/htdocs/modules/system/admin/blocksadmin/main.php
index e28867715..bf581f6e4 100644
--- a/htdocs/modules/system/admin/blocksadmin/main.php
+++ b/htdocs/modules/system/admin/blocksadmin/main.php
@@ -54,8 +54,8 @@
'selvis' => -1
);
foreach ($sel as $key => $value) {
- $_{$key} = isset($_COOKIE[$key]) ? (int)($_COOKIE[$key]) : $value;
- ${$key} = Request::getInt($key, $_{$key}, $method);
+ $default = isset($_COOKIE[$key]) ? (int)($_COOKIE[$key]) : $value;
+ ${$key} = Request::getInt($key, $default, $method);
setcookie($key, ${$key});
}
@@ -81,6 +81,7 @@
// Define scripts
$xoops->theme()->addScript('modules/system/js/admin.js');
$xoops->theme()->addScript('modules/system/js/blocks.js');
+ system_addAjaxToken();
// Define Breadcrumb and tips
$admin_page = new \Xoops\Module\Admin();
$admin_page->addBreadcrumbLink(SystemLocale::CONTROL_PANEL, \XoopsBaseConfig::get('url') . '/admin.php', true);
@@ -203,6 +204,7 @@
$xoops->theme()->addScript('media/jquery/plugins/jquery.form.js');
$xoops->theme()->addScript('modules/system/js/admin.js');
$xoops->theme()->addScript('modules/system/js/blocks.js');
+ system_addAjaxToken();
// Define Breadcrumb and tips
$admin_page = new \Xoops\Module\Admin();
$admin_page->addBreadcrumbLink(SystemLocale::CONTROL_PANEL, \XoopsBaseConfig::get('url') . '/admin.php', true);
@@ -223,6 +225,7 @@
break;
case 'display':
+ $xoops->security()->requireAjaxToken();
// Initialize blocks handler
$block_handler = $xoops->getHandlerBlock();
// Get variable
@@ -238,6 +241,7 @@
break;
case 'drag':
+ $xoops->security()->requireAjaxToken();
// Initialize blocks handler
$block_handler = $xoops->getHandlerBlock();
// Get variable
@@ -253,6 +257,7 @@
break;
case 'order':
+ $xoops->security()->requireAjaxToken();
// Initialize blocks handler
$block_handler = $xoops->getHandlerBlock();
if (isset($_POST['blk'])) {
diff --git a/htdocs/modules/system/admin/extensions/main.php b/htdocs/modules/system/admin/extensions/main.php
index 85a2ea36c..2fb81cda8 100644
--- a/htdocs/modules/system/admin/extensions/main.php
+++ b/htdocs/modules/system/admin/extensions/main.php
@@ -58,6 +58,7 @@
$xoops->theme()->addScript('media/jquery/plugins/jquery.jeditable.js');
$xoops->theme()->addScript('modules/system/js/admin.js');
$xoops->theme()->addScript('modules/system/js/module.js');
+ system_addAjaxToken();
// Define Breadcrumb and tips
$admin_page = new \Xoops\Module\Admin();
$admin_page->addBreadcrumbLink(SystemLocale::CONTROL_PANEL, \XoopsBaseConfig::get('url') . '/admin.php', true);
diff --git a/htdocs/modules/system/admin/modulesadmin/main.php b/htdocs/modules/system/admin/modulesadmin/main.php
index 36e12ed85..b8272cf67 100644
--- a/htdocs/modules/system/admin/modulesadmin/main.php
+++ b/htdocs/modules/system/admin/modulesadmin/main.php
@@ -58,6 +58,7 @@
$xoops->theme()->addScript('media/jquery/plugins/jquery.jeditable.js');
$xoops->theme()->addScript('modules/system/js/admin.js');
$xoops->theme()->addScript('modules/system/js/module.js');
+ system_addAjaxToken();
// Define Breadcrumb and tips
$admin_page = new \Xoops\Module\Admin();
$admin_page->addBreadcrumbLink(SystemLocale::CONTROL_PANEL, \XoopsBaseConfig::get('url') . '/admin.php', true);
@@ -92,22 +93,28 @@
case 'rename':
$xoops->logger()->quiet();
- //$xoops->disableErrorReporting();
+ $xoops->security()->requireAjaxToken();
$mid = Request::getInt('id', 0, 'post');
$value = Request::getString('value', '', 'post');
- if ($mid != 0) {
- $module_handler = $xoops->getHandlerModule();
- $module = $module_handler->getById($mid);
- $module->setVar('name', $value);
- if ($module_handler->insertModule($module)) {
- echo $value;
-
- }
+ $module_handler = $xoops->getHandlerModule();
+ $module = $mid > 0 ? $module_handler->getById($mid) : null;
+ if (!is_object($module)) {
+ http_response_code(404);
+ break;
+ }
+ $module->setVar('name', $value);
+ if (!$module_handler->insertModule($module)) {
+ // an error status makes jEditable restore the old name
+ http_response_code(500);
+ break;
}
+ // jEditable inserts the response as HTML
+ echo htmlspecialchars($value, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8');
break;
case 'order':
+ $xoops->security()->requireAjaxToken();
// Get Module Handler
$module_handler = $xoops->getHandlerModule();
if (isset($_POST['mod'])) {
@@ -131,6 +138,7 @@
case 'active':
$xoops->logger()->quiet();
+ $xoops->security()->requireAjaxToken();
//$xoops->disableErrorReporting();
// Get module handler
$module_handler = $xoops->getHandlerModule();
@@ -159,6 +167,7 @@
case 'display_in_menu':
$xoops->logger()->quiet();
+ $xoops->security()->requireAjaxToken();
//$xoops->disableErrorReporting();
// Get module handler
$module_handler = $xoops->getHandlerModule();
diff --git a/htdocs/modules/system/blocks/main.php b/htdocs/modules/system/blocks/main.php
index 1a1ce1ae5..9ef275fb5 100644
--- a/htdocs/modules/system/blocks/main.php
+++ b/htdocs/modules/system/blocks/main.php
@@ -80,6 +80,7 @@ function b_system_main_edit($options)
// Define scripts
$xoops->theme()->addScript('media/jquery/plugins/jquery.jeditable.js');
$xoops->theme()->addScript('modules/system/js/module.js');
+ system_addAjaxToken();
$admin_page->addTips(SystemLocale::MENU_TIPS);
$admin_page->renderTips();
diff --git a/htdocs/modules/system/include/functions.php b/htdocs/modules/system/include/functions.php
index 92afb5690..a463d340a 100644
--- a/htdocs/modules/system/include/functions.php
+++ b/htdocs/modules/system/include/functions.php
@@ -123,6 +123,25 @@ function system_AdminIcons($img)
return $url;
}
+/**
+ * Give the page's AJAX scripts (admin.js, blocks.js, module.js) the token
+ * their state changes send, as the JS global XOOPS_TOKEN
+ *
+ * @return void
+ */
+function system_addAjaxToken(): void
+{
+ static $added = false;
+ if (!$added) {
+ $xoops = Xoops::getInstance();
+ $token = json_encode($xoops->security()->createToken(3600));
+ $xoops->theme()->addScript('', array('type' => 'text/javascript'), 'var XOOPS_TOKEN = ' . $token . ';');
+ // reloads the page when the token has expired (HTTP 403)
+ $xoops->theme()->addScript('media/xoops/xoops.js');
+ $added = true;
+ }
+}
+
/**
* @param string $name
* @return void
diff --git a/htdocs/modules/system/js/blocks.js b/htdocs/modules/system/js/blocks.js
index 8e88c5a3a..c4400698e 100644
--- a/htdocs/modules/system/js/blocks.js
+++ b/htdocs/modules/system/js/blocks.js
@@ -8,16 +8,16 @@ $(document).ready(
connectWith: '.xo-blocksection',
update: function(event, ui) {
var list = $(this).sortable( 'serialize');
- $.post( 'admin.php?fct=blocksadmin&op=order', list );
+ $.post( 'admin.php?fct=blocksadmin&op=order', list + '&XOOPS_TOKEN_REQUEST=' + XOOPS_TOKEN );
},
receive: function(event, ui) {
var side = $(this).attr('side');
var bid = $(ui.item).attr('bid');
var list = $(this).sortable( 'serialize');
- $.post( 'admin.php', { fct: 'blocksadmin', op: 'drag', bid: bid, side: side } );
+ $.post( 'admin.php', { fct: 'blocksadmin', op: 'drag', bid: bid, side: side, XOOPS_TOKEN_REQUEST: XOOPS_TOKEN } );
- $.post( 'admin.php?fct=blocksadmin&op=order', list );
+ $.post( 'admin.php?fct=blocksadmin&op=order', list + '&XOOPS_TOKEN_REQUEST=' + XOOPS_TOKEN );
}
}
@@ -31,16 +31,16 @@ $(document).ready(
connectWith: '.xo-blocksection'/*,
update: function(event, ui) {
var list = $(this).sortable( 'serialize');
- $.post( 'admin.php?fct=blocksadmin&op=order', list );
+ $.post( 'admin.php?fct=blocksadmin&op=order', list + '&XOOPS_TOKEN_REQUEST=' + XOOPS_TOKEN );
},
receive: function(event, ui) {
var side = $(this).attr('side');
var bid = $(ui.item).attr('bid');
var list = $(this).sortable( 'serialize');
- $.post( 'admin.php', { fct: 'blocksadmin', op: 'drag', bid: bid, side: side } );
+ $.post( 'admin.php', { fct: 'blocksadmin', op: 'drag', bid: bid, side: side, XOOPS_TOKEN_REQUEST: XOOPS_TOKEN } );
- $.post( 'admin.php?fct=blocksadmin&op=order', list );
+ $.post( 'admin.php?fct=blocksadmin&op=order', list + '&XOOPS_TOKEN_REQUEST=' + XOOPS_TOKEN );
}*/
}
diff --git a/htdocs/modules/system/js/module.js b/htdocs/modules/system/js/module.js
index cceb2da6e..a120f47e4 100644
--- a/htdocs/modules/system/js/module.js
+++ b/htdocs/modules/system/js/module.js
@@ -47,7 +47,8 @@ $(function() {
$(".rename").editable("admin.php?fct=modulesadmin&op=rename", {
indicator : "
",
- cssclass : 'span2'
+ cssclass : 'span2',
+ submitdata : function() { return { XOOPS_TOKEN_REQUEST: XOOPS_TOKEN }; }
});
$(".modal-backdrop").click(function(){
@@ -64,7 +65,7 @@ $(function() {
placeholder: 'ui-state-highlight',
update: function(event, ui) {
var list = $(this).sortable( 'serialize');
- $.post( 'admin.php?fct=modulesadmin&op=order', list );
+ $.post( 'admin.php?fct=modulesadmin&op=order', list + '&XOOPS_TOKEN_REQUEST=' + XOOPS_TOKEN );
}}
);
$( "#sortable" ).disableSelection();
@@ -94,7 +95,7 @@ function module_Detail(id){
}
function module_Disable(id, enable, disable){
- $.post( 'admin.php', { fct: 'modulesadmin', op: 'active', mid: id } ,
+ $.post( 'admin.php', { fct: 'modulesadmin', op: 'active', mid: id, XOOPS_TOKEN_REQUEST: XOOPS_TOKEN } ,
function(reponse, textStatus) {
if (textStatus=='success') {
if(reponse){
@@ -110,7 +111,7 @@ function module_Disable(id, enable, disable){
}
function module_Hide(id){
- $.post( 'admin.php', { fct: 'modulesadmin', op: 'display_in_menu', mid: id } ,
+ $.post( 'admin.php', { fct: 'modulesadmin', op: 'display_in_menu', mid: id, XOOPS_TOKEN_REQUEST: XOOPS_TOKEN } ,
function(reponse, textStatus) {
if (textStatus=='success') {
if(reponse != 0){
diff --git a/htdocs/modules/system/templates/admin/system_blocks_item.tpl b/htdocs/modules/system/templates/admin/system_blocks_item.tpl
index 2bc0c2d0f..02ecb78d6 100644
--- a/htdocs/modules/system/templates/admin/system_blocks_item.tpl
+++ b/htdocs/modules/system/templates/admin/system_blocks_item.tpl
@@ -9,7 +9,7 @@

-
diff --git a/htdocs/modules/system/templates/admin/system_index.tpl b/htdocs/modules/system/templates/admin/system_index.tpl
index 6b322d991..24574668f 100644
--- a/htdocs/modules/system/templates/admin/system_index.tpl
+++ b/htdocs/modules/system/templates/admin/system_index.tpl
@@ -29,7 +29,7 @@
<{if $menuitem.used}>
-
+
<{/if}>
|
diff --git a/htdocs/modules/userrank/admin/userrank.php b/htdocs/modules/userrank/admin/userrank.php
index 5f709b518..f8183ed68 100644
--- a/htdocs/modules/userrank/admin/userrank.php
+++ b/htdocs/modules/userrank/admin/userrank.php
@@ -43,6 +43,7 @@
case 'list':
default:
+ $xoops->tpl()->assign('token', $xoops->security()->createToken(3600));
// Add Scripts
$xoops->theme()->addScript('media/xoops/xoops.js');
@@ -178,6 +179,7 @@
// Update userrank status
case 'userrank_update_special':
+ $xoops->security()->requireAjaxToken();
// Get rank id
$rank_id = Request::getInt('rank_id', 0);
if ($rank_id > 0) {
diff --git a/htdocs/modules/userrank/templates/admin/userrank.tpl b/htdocs/modules/userrank/templates/admin/userrank.tpl
index 5a5f70bae..c2ba434b1 100644
--- a/htdocs/modules/userrank/templates/admin/userrank.tpl
+++ b/htdocs/modules/userrank/templates/admin/userrank.tpl
@@ -29,7 +29,7 @@
<{$urank.rank_max}> |
-
+
|
diff --git a/htdocs/user.php b/htdocs/user.php
index 456b7cd91..a2f678e84 100644
--- a/htdocs/user.php
+++ b/htdocs/user.php
@@ -121,6 +121,9 @@
);
$xoops->footer();
} else {
+ if (!$xoops->security()->check()) {
+ $xoops->redirect('user.php', 3, implode(' ', $xoops->security()->getErrors()));
+ }
$del_uid = $xoops->user->getVar("uid");
$member_handler = $xoops->getHandlerMember();
if (false != $member_handler->deleteUser($xoops->user)) {
diff --git a/tests/unit/xoopsLib/Xoops/Core/SecurityTest.php b/tests/unit/xoopsLib/Xoops/Core/SecurityTest.php
index 49930fd0f..a8c97dc31 100644
--- a/tests/unit/xoopsLib/Xoops/Core/SecurityTest.php
+++ b/tests/unit/xoopsLib/Xoops/Core/SecurityTest.php
@@ -10,9 +10,10 @@ class SecurityTest extends \PHPUnit\Framework\TestCase
*/
protected $object;
- protected $SERVER_save;
- protected $SESSION_save;
- protected $moduleConfig_save;
+ protected $savedServer;
+ protected $savedSession;
+ protected $savedPost;
+ protected $savedModuleConfig;
/**
* Sets up the fixture, for example, opens a network connection.
@@ -21,9 +22,10 @@ class SecurityTest extends \PHPUnit\Framework\TestCase
protected function setUp(): void
{
$this->object = new Security();
- $this->SERVER_save = $_SERVER;
- $this->SESSION_save = $_SESSION;
- $this->moduleConfig_save = \Xoops::getInstance()->moduleConfig;
+ $this->savedServer = $_SERVER;
+ $this->savedSession = $_SESSION;
+ $this->savedPost = $_POST;
+ $this->savedModuleConfig = \Xoops::getInstance()->moduleConfig;
}
/**
@@ -32,9 +34,10 @@ protected function setUp(): void
*/
protected function tearDown(): void
{
- $_SERVER = $this->SERVER_save;
- $_SESSION = $this->SESSION_save;
- \Xoops::getInstance()->moduleConfig = $this->moduleConfig_save;
+ $_SERVER = $this->savedServer;
+ $_SESSION = $this->savedSession;
+ $_POST = $this->savedPost;
+ \Xoops::getInstance()->moduleConfig = $this->savedModuleConfig;
}
public function test___construct()
@@ -76,6 +79,69 @@ public function test_check()
$this->assertTrue(empty($_SESSION['XOOPS_TOKEN_SESSION']));
}
+ // a wrongful exit() here would end the whole runner with status 0
+ #[\PHPUnit\Framework\Attributes\RunInSeparateProcess]
+ public function test_requireAjaxToken_keepsTokenForNextToggle()
+ {
+ unset($_SESSION['XOOPS_TOKEN_SESSION']);
+ $token = $this->object->createToken(3600);
+ $_SERVER['REQUEST_METHOD'] = 'POST';
+ $_POST['XOOPS_TOKEN_REQUEST'] = $token;
+
+ // returns (does not exit) for a valid POST, twice with one token
+ $this->object->requireAjaxToken();
+ $this->object->requireAjaxToken();
+ $this->assertSame($token, $_SESSION['XOOPS_TOKEN_SESSION'][0]['id']);
+ }
+
+ public static function rejectedAjaxRequests(): array
+ {
+ return [
+ 'GET with body token' => ['GET', 'valid', false],
+ 'POST without token' => ['POST', null, false],
+ 'POST invalid token' => ['POST', 'nope', false],
+ 'POST expired token' => ['POST', 'valid', true],
+ 'POST token in query' => ['POST', 'query', false],
+ ];
+ }
+
+ #[\PHPUnit\Framework\Attributes\DataProvider('rejectedAjaxRequests')]
+ public function test_checkAjaxToken_rejects(string $method, ?string $token, bool $expired)
+ {
+ unset($_SESSION['XOOPS_TOKEN_SESSION']);
+ $valid = $this->object->createToken(3600);
+ if ($expired) {
+ $_SESSION['XOOPS_TOKEN_SESSION'][0]['expire'] = time() - 1;
+ }
+ $_SERVER['REQUEST_METHOD'] = $method;
+ $savedGet = $_GET;
+ $savedPost = $_POST;
+ $savedRequest = $_REQUEST;
+ unset($_POST['XOOPS_TOKEN_REQUEST'], $_REQUEST['XOOPS_TOKEN_REQUEST']);
+ if ('query' === $token) {
+ $_GET['XOOPS_TOKEN_REQUEST'] = $valid;
+ $_REQUEST['XOOPS_TOKEN_REQUEST'] = $valid;
+ } elseif (null !== $token) {
+ $_POST['XOOPS_TOKEN_REQUEST'] = 'valid' === $token ? $valid : $token;
+ }
+
+ try {
+ $this->assertFalse($this->object->checkAjaxToken());
+ } finally {
+ $_GET = $savedGet;
+ $_POST = $savedPost;
+ $_REQUEST = $savedRequest;
+ }
+ }
+
+ public function test_checkAjaxToken_acceptsValidPost()
+ {
+ unset($_SESSION['XOOPS_TOKEN_SESSION']);
+ $_SERVER['REQUEST_METHOD'] = 'POST';
+ $_POST['XOOPS_TOKEN_REQUEST'] = $this->object->createToken(3600);
+ $this->assertTrue($this->object->checkAjaxToken());
+ }
+
public function test_createToken()
{
$instance = $this->object;
diff --git a/xoops_lib/Xoops/Core/Security.php b/xoops_lib/Xoops/Core/Security.php
index 245e353e1..e15370a6f 100644
--- a/xoops_lib/Xoops/Core/Security.php
+++ b/xoops_lib/Xoops/Core/Security.php
@@ -46,6 +46,42 @@ public function check($clearIfValid = true, $token = false, $name = 'XOOPS_TOKEN
return $this->validateToken($token, $clearIfValid, $name);
}
+ /**
+ * Stop an AJAX state change unless it is a POST carrying a valid token.
+ *
+ * The token is kept, so every toggle on a page can reuse the one token
+ * the page minted (use createToken() with a long timeout for it). A
+ * rejected request ends with HTTP 403, which keeps Xoops.changeStatus()
+ * from flipping its icon.
+ *
+ * @param string $name name of session variable
+ *
+ * @return void
+ */
+ public function requireAjaxToken($name = 'XOOPS_TOKEN'): void
+ {
+ if (!$this->checkAjaxToken($name)) {
+ http_response_code(403);
+ // media/xoops/xoops.js reloads the page on this marker to get a fresh token
+ header('X-Xoops-Token: rejected');
+ exit();
+ }
+ }
+
+ /**
+ * Is this a POST carrying a valid token in its body? The token is kept.
+ *
+ * @param string $name name of session variable
+ *
+ * @return bool
+ */
+ public function checkAjaxToken($name = 'XOOPS_TOKEN'): bool
+ {
+ // the token must come in the body, a query string leaks through logs and referers
+ $token = Request::getString($name . '_REQUEST', '', 'POST');
+ return 'POST' === Request::getMethod() && '' !== $token && $this->check(false, $token, $name);
+ }
+
/**
* Create a token in the user's session
*
|