Skip to content

Commit 1d5e69d

Browse files
TheSerapherTheSerapher
committed
Merge pull request #2227 from MPOS/admin-csrf-protection
[FIX] CSRF protection for admin settings/user/news
2 parents 5d8fecf + 9d0e1f2 commit 1d5e69d

File tree

7 files changed

+67
-52
lines changed

7 files changed

+67
-52
lines changed

‎include/classes/tools.class.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,11 @@ public function getOnlineVersions() {
1818
curl_setopt($curl, CURLOPT_HEADER, false);
1919
$data = curl_exec($curl);
2020
preg_match('/define\(\'MPOS_VERSION\', \'(.*)\'\);/', $data, $match);
21-
$mpos_versions['MPOS_VERSION'] = $match[1];
21+
$mpos_versions['MPOS_VERSION'] = @$match[1];
2222
preg_match('/define\(\'DB_VERSION\', \'(.*)\'\);/', $data, $match);
23-
$mpos_versions['DB_VERSION'] = $match[1];
23+
$mpos_versions['DB_VERSION'] = @$match[1];
2424
preg_match('/define\(\'CONFIG_VERSION\', \'(.*)\'\);/', $data, $match);
25-
$mpos_versions['CONFIG_VERSION'] = $match[1];
25+
$mpos_versions['CONFIG_VERSION'] = @$match[1];
2626
curl_close($curl);
2727
return $this->memcache->setCache($key, $mpos_versions, 30);
2828
} else {

‎include/pages/admin/news.inc.php‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,23 +10,31 @@
1010
// Include markdown library
1111
use \Michelf\Markdown;
1212

13-
if (@$_REQUEST['do'] == 'toggle_active')
14-
if ($news->toggleActive($_REQUEST['id']))
15-
$_SESSION['POPUP'][] = array('CONTENT' => 'News entry changed', 'TYPE' => 'alert alert-success');
13+
if (@$_REQUEST['do'] == 'toggle_active') {
14+
if (!$config['csrf']['enabled'] || $config['csrf']['enabled'] && $csrftoken->valid) {
15+
if ($news->toggleActive($_REQUEST['id'])) {
16+
$_SESSION['POPUP'][] = array('CONTENT' => 'News entry changed', 'TYPE' => 'alert alert-success');
17+
}
18+
}
19+
}
1620

1721
if (@$_REQUEST['do'] == 'add') {
18-
if ($news->addNews($_SESSION['USERDATA']['id'], $_POST['data'])) {
19-
$_SESSION['POPUP'][] = array('CONTENT' => 'News entry added', 'TYPE' => 'alert alert-success');
20-
} else {
21-
$_SESSION['POPUP'][] = array('CONTENT' => 'Failed to add new entry: ' . $news->getError(), 'TYPE' => 'alert alert-danger');
22+
if (!$config['csrf']['enabled'] || $config['csrf']['enabled'] && $csrftoken->valid) {
23+
if ($news->addNews($_SESSION['USERDATA']['id'], $_POST['data'])) {
24+
$_SESSION['POPUP'][] = array('CONTENT' => 'News entry added', 'TYPE' => 'alert alert-success');
25+
} else {
26+
$_SESSION['POPUP'][] = array('CONTENT' => 'Failed to add new entry: ' . $news->getError(), 'TYPE' => 'alert alert-danger');
27+
}
2228
}
2329
}
2430

2531
if (@$_REQUEST['do'] == 'delete') {
26-
if ($news->deleteNews((int)$_REQUEST['id'])) {
27-
$_SESSION['POPUP'][] = array('CONTENT' => 'Succesfully removed news entry', 'TYPE' => 'alert alert-success');
28-
} else {
29-
$_SESSION['POPUP'][] = array('CONTENT' => 'Failed to delete entry: ' . $news->getError(), 'TYPE' => 'alert alert-danger');
32+
if (!$config['csrf']['enabled'] || $config['csrf']['enabled'] && $csrftoken->valid) {
33+
if ($news->deleteNews((int)$_REQUEST['id'])) {
34+
$_SESSION['POPUP'][] = array('CONTENT' => 'Succesfully removed news entry', 'TYPE' => 'alert alert-success');
35+
} else {
36+
$_SESSION['POPUP'][] = array('CONTENT' => 'Failed to delete entry: ' . $news->getError(), 'TYPE' => 'alert alert-danger');
37+
}
3038
}
3139
}
3240

@@ -38,4 +46,4 @@
3846
}
3947
$smarty->assign("NEWS", $aNews);
4048
$smarty->assign("CONTENT", "default.tpl");
41-
?>
49+
?>

‎include/pages/admin/news_edit.inc.php‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,18 @@
1010
// Include markdown library
1111
use \Michelf\Markdown;
1212

13-
if (@$_REQUEST['do'] == 'save') {
14-
if ($news->updateNews($_REQUEST['id'], $_REQUEST['header'], $_REQUEST['content'], $_REQUEST['active'])) {
15-
$_SESSION['POPUP'][] = array('CONTENT' => 'News updated', 'TYPE' => 'alert alert-success');
16-
} else {
17-
$_SESSION['POPUP'][] = array('CONTENT' => 'News update failed: ' . $news->getError(), 'TYPE' => 'alert alert-danger');
13+
if (!$config['csrf']['enabled'] || $config['csrf']['enabled'] && $csrftoken->valid) {
14+
if (@$_REQUEST['do'] == 'save') {
15+
if ($news->updateNews($_REQUEST['id'], $_REQUEST['header'], $_REQUEST['content'], $_REQUEST['active'])) {
16+
$_SESSION['POPUP'][] = array('CONTENT' => 'News updated', 'TYPE' => 'alert alert-success');
17+
} else {
18+
$_SESSION['POPUP'][] = array('CONTENT' => 'News update failed: ' . $news->getError(), 'TYPE' => 'alert alert-danger');
19+
}
1820
}
1921
}
2022

2123
// Fetch news entry
2224
$aNews = $news->getEntry($_REQUEST['id']);
2325
$smarty->assign("NEWS", $aNews);
2426
$smarty->assign("CONTENT", "default.tpl");
25-
?>
27+
?>

‎include/pages/admin/settings.inc.php‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,15 @@
88
}
99

1010
if (@$_REQUEST['do'] == 'save' && !empty($_REQUEST['data'])) {
11-
$user->log->log("warn", @$_SESSION['USERDATA']['username']." changed admin settings");
12-
foreach($_REQUEST['data'] as $var => $value) {
13-
$setting->setValue($var, $value);
11+
if (!$config['csrf']['enabled'] || $config['csrf']['enabled'] && $csrftoken->valid) {
12+
$user->log->log("warn", @$_SESSION['USERDATA']['username']." changed admin settings");
13+
foreach($_REQUEST['data'] as $var => $value) {
14+
$setting->setValue($var, $value);
15+
}
16+
$_SESSION['POPUP'][] = array('CONTENT' => 'Settings updated', 'TYPE' => 'alert alert-success');
17+
} else {
18+
$_SESSION['POPUP'][] = array('CONTENT' => $csrftoken->getErrorWithDescriptionHTML(), 'TYPE' => 'alert alert-warning');
1419
}
15-
$_SESSION['POPUP'][] = array('CONTENT' => 'Settings updated', 'TYPE' => 'alert alert-success');
1620
}
1721

1822
// Load our available settings from configuration
@@ -23,4 +27,4 @@
2327

2428
// Tempalte specifics
2529
$smarty->assign("CONTENT", "default.tpl");
26-
?>
30+
?>

‎include/pages/admin/user.inc.php‎

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,26 +16,28 @@
1616
$smarty->assign('NOFEE', array('' => '', '0' => 'No', '1' => 'Yes'));
1717

1818
// Catch our JS queries to update some settings
19-
switch (@$_REQUEST['do']) {
20-
case 'lock':
21-
$supress_master = 1;
22-
// Reset user account
23-
if ($user->isLocked($_POST['account_id']) == 0) {
24-
$user->setLocked($_POST['account_id'], 2);
25-
} else {
26-
$user->setLocked($_POST['account_id'], 0);
27-
$user->setUserFailed($_POST['account_id'], 0);
28-
$user->setUserPinFailed($_POST['account_id'], 0);
19+
if (!$config['csrf']['enabled'] || $config['csrf']['enabled'] && $csrftoken->valid) {
20+
switch (@$_REQUEST['do']) {
21+
case 'lock':
22+
$supress_master = 1;
23+
// Reset user account
24+
if ($user->isLocked($_POST['account_id']) == 0) {
25+
$user->setLocked($_POST['account_id'], 2);
26+
} else {
27+
$user->setLocked($_POST['account_id'], 0);
28+
$user->setUserFailed($_POST['account_id'], 0);
29+
$user->setUserPinFailed($_POST['account_id'], 0);
30+
}
31+
break;
32+
case 'fee':
33+
$supress_master = 1;
34+
$user->changeNoFee($_POST['account_id']);
35+
break;
36+
case 'admin':
37+
$supress_master = 1;
38+
$user->changeAdmin($_POST['account_id']);
39+
break;
2940
}
30-
break;
31-
case 'fee':
32-
$supress_master = 1;
33-
$user->changeNoFee($_POST['account_id']);
34-
break;
35-
case 'admin':
36-
$supress_master = 1;
37-
$user->changeAdmin($_POST['account_id']);
38-
break;
3941
}
4042

4143
// Gernerate the GET URL for filters
@@ -81,4 +83,4 @@
8183

8284
// Tempalte specifics
8385
$smarty->assign("CONTENT", "default.tpl");
84-
?>
86+
?>

‎templates/bootstrap/admin/news/default.tpl‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,11 @@
5353
<div class="panel-footer">
5454
<div style="text-align:right">
5555
<a href='{$smarty.server.SCRIPT_NAME}?page={$smarty.request.page|escape}&action=news_edit&id={$NEWS[news].id}'><i class="fa fa-wrench fa-fw"></i></a>&nbsp;
56-
<a href='{$smarty.server.SCRIPT_NAME}?page={$smarty.request.page|escape}&action={$smarty.request.action|escape}&do=delete&id={$NEWS[news].id}'><i class="fa fa-trash-o fa-fw"></i></a>
56+
<a href='{$smarty.server.SCRIPT_NAME}?page={$smarty.request.page|escape}&action={$smarty.request.action|escape}&do=delete&id={$NEWS[news].id}&ctoken={$CTOKEN|escape|default:""}'><i class="fa fa-trash-o fa-fw"></i></a>
5757
</div>
5858
</div>
5959
</div>
6060
</div>
6161
{/section}
6262
{/nocache}
6363
</div>
64-

‎templates/bootstrap/admin/user/default.tpl‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,21 @@
33
$.ajax({
44
type: "POST",
55
url: "{$smarty.server.SCRIPT_NAME}",
6-
data: "page={$smarty.request.page|escape}&action={$smarty.request.action|escape}&do=fee&account_id=" + id,
6+
data: "page={$smarty.request.page|escape}&action={$smarty.request.action|escape}&do=fee&account_id=" + id + "&ctoken={$smarty.request.ctoken|escape}",
77
});
88
}
99
function storeLock(id) {
1010
$.ajax({
1111
type: "POST",
1212
url: "{$smarty.server.SCRIPT_NAME}",
13-
data: "page={$smarty.request.page|escape}&action={$smarty.request.action|escape}&do=lock&account_id=" + id,
13+
data: "page={$smarty.request.page|escape}&action={$smarty.request.action|escape}&do=lock&account_id=" + id + "&ctoken={$smarty.request.ctoken|escape}",
1414
});
1515
}
1616
function storeAdmin(id) {
1717
$.ajax({
1818
type: "POST",
1919
url: "{$smarty.server.SCRIPT_NAME}",
20-
data: "page={$smarty.request.page|escape}&action={$smarty.request.action|escape}&do=admin&account_id=" + id,
20+
data: "page={$smarty.request.page|escape}&action={$smarty.request.action|escape}&do=admin&account_id=" + id + "&ctoken={$smarty.request.ctoken|escape}",
2121
});
2222
}
2323
</script>
@@ -147,4 +147,4 @@
147147
</div>
148148
</div>
149149
</div>
150-
</div>
150+
</div>

0 commit comments

Comments
 (0)