Merge pull request #2227 from MPOS/admin-csrf-protection

[FIX] CSRF protection for admin settings/user/news
2014-06-10 15:08:34 +02:00 · 2014-06-10 15:08:34 +02:00 · 1d5e69d219
commit 1d5e69d219
parent 5d8fecfd81 9d0e1f2ba5
7 changed files with 67 additions and 52 deletions
--- a/include/classes/tools.class.php
+++ b/include/classes/tools.class.php
@ -18,11 +18,11 @@ class Tools extends Base {
      curl_setopt($curl, CURLOPT_HEADER, false);
      $data = curl_exec($curl);
      preg_match('/define\(\'MPOS_VERSION\', \'(.*)\'\);/', $data, $match);
-      $mpos_versions['MPOS_VERSION'] = $match[1];
+      $mpos_versions['MPOS_VERSION'] = @$match[1];
      preg_match('/define\(\'DB_VERSION\', \'(.*)\'\);/', $data, $match);
-      $mpos_versions['DB_VERSION'] = $match[1];
+      $mpos_versions['DB_VERSION'] = @$match[1];
      preg_match('/define\(\'CONFIG_VERSION\', \'(.*)\'\);/', $data, $match);
-      $mpos_versions['CONFIG_VERSION'] = $match[1];
+      $mpos_versions['CONFIG_VERSION'] = @$match[1];
      curl_close($curl);
      return $this->memcache->setCache($key, $mpos_versions, 30);
    } else {
--- a/include/pages/admin/news.inc.php
+++ b/include/pages/admin/news.inc.php
@ -10,23 +10,31 @@ if (!$user->isAuthenticated() || !$user->isAdmin($_SESSION['USERDATA']['id'])) {
 // Include markdown library
 use \Michelf\Markdown;

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

 if (@$_REQUEST['do'] == 'add') {
-  if ($news->addNews($_SESSION['USERDATA']['id'], $_POST['data'])) {
-    $_SESSION['POPUP'][] = array('CONTENT' => 'News entry added', 'TYPE' => 'alert alert-success');
-  } else {
-    $_SESSION['POPUP'][] = array('CONTENT' => 'Failed to add new entry: ' . $news->getError(), 'TYPE' => 'alert alert-danger');
+  if (!$config['csrf']['enabled'] || $config['csrf']['enabled'] && $csrftoken->valid) {
+    if ($news->addNews($_SESSION['USERDATA']['id'], $_POST['data'])) {
+      $_SESSION['POPUP'][] = array('CONTENT' => 'News entry added', 'TYPE' => 'alert alert-success');
+    } else {
+      $_SESSION['POPUP'][] = array('CONTENT' => 'Failed to add new entry: ' . $news->getError(), 'TYPE' => 'alert alert-danger');
+    }
  }
 }

 if (@$_REQUEST['do'] == 'delete') {
-  if ($news->deleteNews((int)$_REQUEST['id'])) {
-    $_SESSION['POPUP'][] = array('CONTENT' => 'Succesfully removed news entry', 'TYPE' => 'alert alert-success');
-  } else {
-    $_SESSION['POPUP'][] = array('CONTENT' => 'Failed to delete entry: ' . $news->getError(), 'TYPE' => 'alert alert-danger');
+  if (!$config['csrf']['enabled'] || $config['csrf']['enabled'] && $csrftoken->valid) {
+    if ($news->deleteNews((int)$_REQUEST['id'])) {
+      $_SESSION['POPUP'][] = array('CONTENT' => 'Succesfully removed news entry', 'TYPE' => 'alert alert-success');
+    } else {
+      $_SESSION['POPUP'][] = array('CONTENT' => 'Failed to delete entry: ' . $news->getError(), 'TYPE' => 'alert alert-danger');
+    }
  }
 }

@ -38,4 +46,4 @@ foreach ($aNews as $key => $aData) {
 }
 $smarty->assign("NEWS", $aNews);
 $smarty->assign("CONTENT", "default.tpl");
-?>
+?>
--- a/include/pages/admin/news_edit.inc.php
+++ b/include/pages/admin/news_edit.inc.php
@ -10,11 +10,13 @@ if (!$user->isAuthenticated() || !$user->isAdmin($_SESSION['USERDATA']['id'])) {
 // Include markdown library
 use \Michelf\Markdown;

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

@ -22,4 +24,4 @@ if (@$_REQUEST['do'] == 'save') {
 $aNews = $news->getEntry($_REQUEST['id']);
 $smarty->assign("NEWS", $aNews);
 $smarty->assign("CONTENT", "default.tpl");
-?>
+?>
--- a/include/pages/admin/settings.inc.php
+++ b/include/pages/admin/settings.inc.php
@ -8,11 +8,15 @@ if (!$user->isAuthenticated() || !$user->isAdmin($_SESSION['USERDATA']['id'])) {
 }

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

 // Load our available settings from configuration
@ -23,4 +27,4 @@ $smarty->assign("SETTINGS", $aSettings);

 // Tempalte specifics
 $smarty->assign("CONTENT", "default.tpl");
-?>
+?>
--- a/include/pages/admin/user.inc.php
+++ b/include/pages/admin/user.inc.php
@ -16,26 +16,28 @@ $smarty->assign('LOCKED', array('' => '', '0' => 'No', '1' => 'Yes'));
 $smarty->assign('NOFEE', array('' => '', '0' => 'No', '1' => 'Yes'));

 // Catch our JS queries to update some settings
-switch (@$_REQUEST['do']) {
-case 'lock':
-  $supress_master = 1;
-  // Reset user account
-  if ($user->isLocked($_POST['account_id']) == 0) {
-    $user->setLocked($_POST['account_id'], 2);
-  } else {
-    $user->setLocked($_POST['account_id'], 0);
-    $user->setUserFailed($_POST['account_id'], 0);
-    $user->setUserPinFailed($_POST['account_id'], 0);
+if (!$config['csrf']['enabled'] || $config['csrf']['enabled'] && $csrftoken->valid) {
+  switch (@$_REQUEST['do']) {
+  case 'lock':
+    $supress_master = 1;
+    // Reset user account
+    if ($user->isLocked($_POST['account_id']) == 0) {
+      $user->setLocked($_POST['account_id'], 2);
+    } else {
+      $user->setLocked($_POST['account_id'], 0);
+      $user->setUserFailed($_POST['account_id'], 0);
+      $user->setUserPinFailed($_POST['account_id'], 0);
+    }
+    break;
+  case 'fee':
+    $supress_master = 1;
+    $user->changeNoFee($_POST['account_id']);
+    break;
+  case 'admin':
+    $supress_master = 1;
+    $user->changeAdmin($_POST['account_id']);
+    break;
  }
-  break;
-case 'fee':
-  $supress_master = 1;
-  $user->changeNoFee($_POST['account_id']);
-  break;
-case 'admin':
-  $supress_master = 1;
-  $user->changeAdmin($_POST['account_id']);
-  break;
 }

 // Gernerate the GET URL for filters
@ -81,4 +83,4 @@ if (isset($_REQUEST['filter'])) {

 // Tempalte specifics
 $smarty->assign("CONTENT", "default.tpl");
-?>
+?>
--- a/templates/bootstrap/admin/news/default.tpl
+++ b/templates/bootstrap/admin/news/default.tpl
@ -53,7 +53,7 @@
      <div class="panel-footer">
        <div style="text-align:right">
          <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;
-          <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>
+          <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>
        </div>
      </div>
    </div>
@ -61,4 +61,3 @@
 {/section}
 {/nocache}
 </div>
-
--- a/templates/bootstrap/admin/user/default.tpl
+++ b/templates/bootstrap/admin/user/default.tpl
@ -3,21 +3,21 @@
      $.ajax({
       type: "POST",
       url: "{$smarty.server.SCRIPT_NAME}",
-       data: "page={$smarty.request.page|escape}&action={$smarty.request.action|escape}&do=fee&account_id=" + id,
+       data: "page={$smarty.request.page|escape}&action={$smarty.request.action|escape}&do=fee&account_id=" + id + "&ctoken={$smarty.request.ctoken|escape}",
     });
    }
    function storeLock(id) {
      $.ajax({
       type: "POST",
       url: "{$smarty.server.SCRIPT_NAME}",
-       data: "page={$smarty.request.page|escape}&action={$smarty.request.action|escape}&do=lock&account_id=" + id,
+       data: "page={$smarty.request.page|escape}&action={$smarty.request.action|escape}&do=lock&account_id=" + id + "&ctoken={$smarty.request.ctoken|escape}",
     });
    }
    function storeAdmin(id) {
      $.ajax({
       type: "POST",
       url: "{$smarty.server.SCRIPT_NAME}",
-       data: "page={$smarty.request.page|escape}&action={$smarty.request.action|escape}&do=admin&account_id=" + id,
+       data: "page={$smarty.request.page|escape}&action={$smarty.request.action|escape}&do=admin&account_id=" + id + "&ctoken={$smarty.request.ctoken|escape}",
     });
    }
 </script>
@ -147,4 +147,4 @@
      </div>
    </div>
  </div>
-</div>
+</div>