CVE-2022-34134: Cross-Site Request Forgery (CSRF) (CWE-352)
value: Benjamin BALET Jorani v1.0 was discovered to contain a Cross-Site Request Forgery (CSRF) via the component /application/controllers/Users.php.
CWE description: The web application does not, or can not, sufficiently verify whether a well-formed, valid, consistent request was intentionally provided by the user who submitted the request.
CVSS v3 base score: 8.8
Published: 2022-06-28T00:15Z
Repository: https://github.com/bbalet/jorani
Language: PHP
Files changed: application/config/routes.php, application/controllers/Users.php, application/views/users/index.php
Commit date: 2022-06-06 17:29:03 +0200
Commit message: BF:security on users mgt with CSRF token fix #369

Diff: diff --git a/application/config/routes.php b/application/config/routes.php
index d464ebc0..8a3ec44e 100644
--- a/application/config/routes.php
+++ b/application/config/routes.php
@@ -56,14 +56,12 @@ $route['users/employees'] = 'users/employees';
 $route['users/employeesMultiSelect'] = 'users/employeesMultiSelect';
 $route['users/export'] = 'users/export';
 $route['users/reset/(:num)'] = 'users/reset/$1';
 $route['users/create'] = 'users/create';
 $route['users/edit/(:num)'] = 'users/edit/$1';
-$route['users/delete/(:num)'] = 'users/delete/$1';
 $route['users/check/login'] = 'users/checkLoginByAjax';
-$route['users/enable/(:num)'] = 'users/enable/$1';
-$route['users/disable/(:num)'] = 'users/disable/$1';
+$route['users/account']['POST'] = 'users/account';
 $route['users'] = 'users';
 
 //_______________________________________________
 //Human Resources Management
 $route['hr/employees'] = 'hr/employees';
diff --git a/application/controllers/Users.php b/application/controllers/Users.php
index c59103fa..f35b3847 100644
--- a/application/controllers/Users.php
+++ b/application/controllers/Users.php
@@ -44,38 +44,38 @@ class Users extends CI_Controller {
         $this->load->view('users/index', $data);
         $this->load->view('templates/footer');
     }
 
     /**
-     * Set a user as active (TRUE) or inactive (FALSE)
-     * @param int $id User identifier
-     * @param bool $active active (TRUE) or inactive (FALSE)
+     * Account management (activate/disable/delete) is done by a 
+     * POST request with a CSRF token for an improved security
      * @author Benjamin BALET <benjamin.balet@gmail.com>
      */
-    public function active($id, $active) {
+    public function account() {
         $this->auth->checkIfOperationIsAllowed('list_users');
-        $this->users_model->setActive($id, $active);
-        $this->session->set_flashdata('msg', lang('users_edit_flash_msg_success'));
-        redirect('users');
-    }
-
-    /**
-     * Enable a user
-     * @param int $id User identifier
-     * @author Benjamin BALET <benjamin.balet@gmail.com>
-     */
-    public function enable($id) {
-        $this->active($id, TRUE);
-    }
-
-    /**
-     * Disable a user
-     * @param int $id User identifier
-     * @author Benjamin BALET <benjamin.balet@gmail.com>
-     */
-    public function disable($id) {
-        $this->active($id, FALSE);
+        $id = $this->input->post('id');
+        $operation = $this->input->post('operation');
+        //Test if user exists
+        $data['users_item'] = $this->users_model->getUsers($id);
+        if (empty($data['users_item'])) {
+            redirect('notfound');
+        } else {
+            switch ($operation) {
+                case 'enable': 
+                    $this->users_model->setActive($id, TRUE);
+                    log_message('error', 'User #' . $id . ' has been enabled by user #' . $this->session->userdata('id'));
+                    break;
+                case 'disable':
+                    $this->users_model->setActive($id, FALSE);
+                    log_message('error', 'User #' . $id . ' has been disabled by user #' . $this->session->userdata('id'));
+                    break;
+                case 'delete':
+                    $this->auth->checkIfOperationIsAllowed('delete_user');
+                    $this->users_model->deleteUser($id);
+                    log_message('error', 'User #' . $id . ' has been deleted by user #' . $this->session->userdata('id'));
+            }
+        }
     }
 
     /**
      * Display the modal pop-up content of the list of employees
      * @author Benjamin BALET <benjamin.balet@gmail.com>
@@ -192,29 +192,10 @@ class Users extends CI_Controller {
                 redirect('users');
             }
         }
     }
 
-    /**
-     * Delete a user. Log it as an error.
-     * @param int $id User identifier
-     * @author Benjamin BALET <benjamin.balet@gmail.com>
-     */
-    public function delete($id) {
-        $this->auth->checkIfOperationIsAllowed('delete_user');
-        //Test if user exists
-        $data['users_item'] = $this->users_model->getUsers($id);
-        if (empty($data['users_item'])) {
-            redirect('notfound');
-        } else {
-            $this->users_model->deleteUser($id);
-        }
-        log_message('error', 'User #' . $id . ' has been deleted by user #' . $this->session->userdata('id'));
-        $this->session->set_flashdata('msg', lang('users_delete_flash_msg_success'));
-        redirect('users');
-    }
-
     /**
      * Reset the password of a user
      * Can be accessed by the user itself or by admin
      * @param int $id User identifier
      * @author Benjamin BALET <benjamin.balet@gmail.com>
diff --git a/application/views/users/index.php b/application/views/users/index.php
index 47444fd9..be4d846b 100644
--- a/application/views/users/index.php
+++ b/application/views/users/index.php
@@ -26,24 +26,28 @@
             <th><?php echo lang('users_index_thead_role');?></th>
         </tr>
     </thead>
     <tbody>
 <?php foreach ($users as $users_item): ?>
-    <tr>
+    <tr data-id="<?php echo $users_item['id']; ?>">
         <td data-order="<?php echo $users_item['id']; ?>">
             <?php echo $users_item['id'] ?>&nbsp;
             <div class="pull-right">
+            <?php if ($users_item['id'] != $this->session->userdata('id')) { ?>
                 <?php if ($users_item['active']) { ?>
-                <a href="<?php echo base_url();?>users/disable/<?php echo $users_item['id'] ?>" title="<?php echo lang('users_index_thead_tip_active');?>"><i class="mdi mdi-account-off nolink"></i></a>
+                <a href="#" class="action-disable" data-id="<?php echo $users_item['id'];?>" title="<?php echo lang('users_index_thead_tip_active');?>"><i class="mdi mdi-account-off nolink"></i></a>
                 <?php } else { ?>
-                <a href="<?php echo base_url();?>users/enable/<?php echo $users_item['id'] ?>" title="<?php echo lang('users_index_thead_tip_inactive');?>"><i class="mdi mdi-account mdi-dark mdi-inactive"></i></a>
+                <a href="#" class="action-enable" data-id="<?php echo $users_item['id'];?>" title="<?php echo lang('users_index_thead_tip_inactive');?>"><i class="mdi mdi-account mdi-dark mdi-inactive"></i></a>
                 <?php } ?>
+            <?php } ?>
                 &nbsp;
                 <a href="<?php echo base_url();?>users/edit/<?php echo $users_item['id'] ?>" title="<?php echo lang('users_index_thead_tip_edit');?>"><i class="mdi mdi-account-edit nolink"></i></a>
                 &nbsp;
+                <?php if ($users_item['id'] != $this->session->userdata('id')) { ?>
                 <a href="#" class="confirm-delete" data-id="<?php echo $users_item['id'];?>" title="<?php echo lang('users_index_thead_tip_delete');?>"><i class="mdi mdi-delete nolink"></i></a>
                 &nbsp;
+                <?php } ?>
                 <a href="<?php echo base_url();?>users/reset/<?php echo $users_item['id'] ?>" title="<?php echo lang('users_index_thead_tip_reset');?>" data-target="#frmResetPwd" data-toggle="modal"><i class="mdi mdi-lock nolink"></i></a>
             </div>
         </td>
         <td><?php echo $users_item['firstname']; ?></td>
         <td><?php echo $users_item['lastname']; ?></td>
@@ -77,11 +81,11 @@
     <div class="modal-body">
         <p><?php echo lang('users_index_popup_delete_message');?></p>
         <p><?php echo lang('users_index_popup_delete_question');?></p>
     </div>
     <div class="modal-footer">
-        <a href="#" class="btn btn-danger" id="lnkDeleteUser"><?php echo lang('users_index_popup_delete_button_yes');?></a>
+        <a href="#" class="btn btn-danger" id="action-delete"><?php echo lang('users_index_popup_delete_button_yes');?></a>
         <a href="#" onclick="$('#frmConfirmDelete').modal('hide');" class="btn"><?php echo lang('users_index_popup_delete_button_no');?></a>
     </div>
 </div>
 
 <div id="frmResetPwd" class="modal hide fade">
@@ -117,15 +121,38 @@
 
 </div>
 
 <link href="<?php echo base_url();?>assets/datatable/DataTables-1.10.11/css/jquery.dataTables.min.css" rel="stylesheet">
 <script type="text/javascript" src="<?php echo base_url();?>assets/datatable/DataTables-1.10.11/js/jquery.dataTables.min.js"></script>
+<script src="<?php echo base_url();?>assets/js/bootbox.min.js"></script>
 
 <script type="text/javascript">
 $(document).ready(function() {
+
+    <?php if ($this->config->item('csrf_protection') == TRUE) {?>
+    $.ajaxSetup({
+        data: {
+            <?php echo $this->security->get_csrf_token_name();?>: "<?php echo $this->security->get_csrf_hash();?>",
+        }
+    });
+    <?php }?>
+
+    //Global Ajax error handling mainly used for session expiration
+    $( document ).ajaxError(function(event, jqXHR, settings, errorThrown) {
+        $('#frmModalAjaxWait').modal('hide');
+        if (jqXHR.status == 401) {
+            bootbox.alert("<?php echo lang('global_ajax_timeout');?>", function() {
+                //After the login page, we'll be redirected to the current page
+                location.reload();
+            });
+        } else { //Oups
+            bootbox.alert("<?php echo lang('global_ajax_error');?>");
+        }
+    });
+
     //Transform the HTML table in a fancy datatable
-    $('#users').dataTable({
+    oTable = $('#users').DataTable({
         stateSave: true,
         language: {
             decimal:            "<?php echo lang('datatable_sInfoThousands');?>",
             processing:       "<?php echo lang('datatable_sProcessing');?>",
             search:              "<?php echo lang('datatable_sSearch');?>",
@@ -150,24 +177,54 @@ $(document).ready(function() {
         },
     });
     $("#frmResetPwd").alert();
     $("#frmImportUsers").alert();
 
-    //On showing the confirmation pop-up, add the user id at the end of the delete url action
+    //On showing the confirmation pop-up, add the user id as an attribute of the delete url link
     $('#frmConfirmDelete').on('show', function() {
-        var link = "<?php echo base_url();?>users/delete/" + $(this).data('id');
-        $("#lnkDeleteUser").attr('href', link);
+        $("#action-delete").attr('data-id', $(this).data('id'));
     });
 
     //Display a modal pop-up so as to confirm if a user has to be deleted or not
     //We build a complex selector because datatable does horrible things on DOM...
     //a simplier selector doesn't work when the delete is on page >1
     $("#users tbody").on('click', '.confirm-delete',  function(){
         var id = $(this).data('id');
         $('#frmConfirmDelete').data('id', id).modal('show');
     });
 
+    //Enable a user
+    $("#users tbody").on('click', '.action-enable',  function(){
+        var id = $(this).data('id');
+        var ref = $(this);
+        $.post( "<?php echo base_url();?>users/account", { operation: "enable", id: id }).done(function() {
+            ref.attr('class', 'action-disable');
+            ref.attr('title', '<?php echo lang('users_index_thead_tip_active');?>');
+            ref.children(":first").attr('class', 'mdi mdi-account-off nolink');
+        });
+    });
+
+    //Disable a user
+    $("#users tbody").on('click', '.action-disable',  function(){
+        var id = $(this).data('id');
+        var ref = $(this);
+        $.post( "<?php echo base_url();?>users/account", { operation: "disable", id: id }).done(function() {
+            ref.attr('class', 'action-enable');
+            ref.attr('title', '<?php echo lang('users_index_thead_tip_inactive');?>');
+            ref.children(":first").attr('class', 'mdi mdi-account mdi-dark mdi-inactive');
+        });
+    });
+
+    //Delete a user
+    $("#action-delete").on('click',  function(){
+        var id = $(this).data('id');
+        $.post( "<?php echo base_url();?>users/account", { operation: "delete", id: id }).done(function() {
+            oTable.rows('tr[data-id="' + id + '"]').remove().draw();
+            $('#frmConfirmDelete').modal('hide');
+        });
+    });
+
     //Prevent to load always the same content (refreshed each time)
     $('#frmConfirmDelete').on('hidden', function() {
         $(this).removeData('modal');
     });
     $('#frmResetPwd').on('hidden', function() {

Vulnerable code: $route['users/delete/(:num)'] = 'users/delete/$1';
$route['users/enable/(:num)'] = 'users/enable/$1';
$route['users/disable/(:num)'] = 'users/disable/$1';
     * Set a user as active (TRUE) or inactive (FALSE)
     * @param int $id User identifier
     * @param bool $active active (TRUE) or inactive (FALSE)
    public function active($id, $active) {
        $this->users_model->setActive($id, $active);
        $this->session->set_flashdata('msg', lang('users_edit_flash_msg_success'));
        redirect('users');
    }

    /**
     * Enable a user
     * @param int $id User identifier
     * @author Benjamin BALET <benjamin.balet@gmail.com>
     */
    public function enable($id) {
        $this->active($id, TRUE);
    }

    /**
     * Disable a user
     * @param int $id User identifier
     * @author Benjamin BALET <benjamin.balet@gmail.com>
     */
    public function disable($id) {
        $this->active($id, FALSE);
    /**
     * Delete a user. Log it as an error.
     * @param int $id User identifier
     * @author Benjamin BALET <benjamin.balet@gmail.com>
     */
    public function delete($id) {
        $this->auth->checkIfOperationIsAllowed('delete_user');
        //Test if user exists
        $data['users_item'] = $this->users_model->getUsers($id);
        if (empty($data['users_item'])) {
            redirect('notfound');
        } else {
            $this->users_model->deleteUser($id);
        }
        log_message('error', 'User #' . $id . ' has been deleted by user #' . $this->session->userdata('id'));
        $this->session->set_flashdata('msg', lang('users_delete_flash_msg_success'));
        redirect('users');
    }

    <tr>
                <a href="<?php echo base_url();?>users/disable/<?php echo $users_item['id'] ?>" title="<?php echo lang('users_index_thead_tip_active');?>"><i class="mdi mdi-account-off nolink"></i></a>
                <a href="<?php echo base_url();?>users/enable/<?php echo $users_item['id'] ?>" title="<?php echo lang('users_index_thead_tip_inactive');?>"><i class="mdi mdi-account mdi-dark mdi-inactive"></i></a>
        <a href="#" class="btn btn-danger" id="lnkDeleteUser"><?php echo lang('users_index_popup_delete_button_yes');?></a>
    $('#users').dataTable({
    //On showing the confirmation pop-up, add the user id at the end of the delete url action
        var link = "<?php echo base_url();?>users/delete/" + $(this).data('id');
        $("#lnkDeleteUser").attr('href', link);
Fixed code: $route['users/account']['POST'] = 'users/account';
     * Account management (activate/disable/delete) is done by a 
     * POST request with a CSRF token for an improved security
    public function account() {
        $id = $this->input->post('id');
        $operation = $this->input->post('operation');
        //Test if user exists
        $data['users_item'] = $this->users_model->getUsers($id);
        if (empty($data['users_item'])) {
            redirect('notfound');
        } else {
            switch ($operation) {
                case 'enable': 
                    $this->users_model->setActive($id, TRUE);
                    log_message('error', 'User #' . $id . ' has been enabled by user #' . $this->session->userdata('id'));
                    break;
                case 'disable':
                    $this->users_model->setActive($id, FALSE);
                    log_message('error', 'User #' . $id . ' has been disabled by user #' . $this->session->userdata('id'));
                    break;
                case 'delete':
                    $this->auth->checkIfOperationIsAllowed('delete_user');
                    $this->users_model->deleteUser($id);
                    log_message('error', 'User #' . $id . ' has been deleted by user #' . $this->session->userdata('id'));
            }
        }
    <tr data-id="<?php echo $users_item['id']; ?>">
            <?php if ($users_item['id'] != $this->session->userdata('id')) { ?>
                <a href="#" class="action-disable" data-id="<?php echo $users_item['id'];?>" title="<?php echo lang('users_index_thead_tip_active');?>"><i class="mdi mdi-account-off nolink"></i></a>
                <a href="#" class="action-enable" data-id="<?php echo $users_item['id'];?>" title="<?php echo lang('users_index_thead_tip_inactive');?>"><i class="mdi mdi-account mdi-dark mdi-inactive"></i></a>
            <?php } ?>
                <?php if ($users_item['id'] != $this->session->userdata('id')) { ?>
                <?php } ?>
        <a href="#" class="btn btn-danger" id="action-delete"><?php echo lang('users_index_popup_delete_button_yes');?></a>
<script src="<?php echo base_url();?>assets/js/bootbox.min.js"></script>

    <?php if ($this->config->item('csrf_protection') == TRUE) {?>
    $.ajaxSetup({
        data: {
            <?php echo $this->security->get_csrf_token_name();?>: "<?php echo $this->security->get_csrf_hash();?>",
        }
    });
    <?php }?>

    //Global Ajax error handling mainly used for session expiration
    $( document ).ajaxError(function(event, jqXHR, settings, errorThrown) {
        $('#frmModalAjaxWait').modal('hide');
        if (jqXHR.status == 401) {
            bootbox.alert("<?php echo lang('global_ajax_timeout');?>", function() {
                //After the login page, we'll be redirected to the current page
                location.reload();
            });
        } else { //Oups
            bootbox.alert("<?php echo lang('global_ajax_error');?>");
        }
    });

    oTable = $('#users').DataTable({
    //On showing the confirmation pop-up, add the user id as an attribute of the delete url link
        $("#action-delete").attr('data-id', $(this).data('id'));
    //Enable a user
    $("#users tbody").on('click', '.action-enable',  function(){
        var id = $(this).data('id');
        var ref = $(this);
        $.post( "<?php echo base_url();?>users/account", { operation: "enable", id: id }).done(function() {
            ref.attr('class', 'action-disable');
            ref.attr('title', '<?php echo lang('users_index_thead_tip_active');?>');
            ref.children(":first").attr('class', 'mdi mdi-account-off nolink');
        });
    });

    //Disable a user
    $("#users tbody").on('click', '.action-disable',  function(){
        var id = $(this).data('id');
        var ref = $(this);
        $.post( "<?php echo base_url();?>users/account", { operation: "disable", id: id }).done(function() {
            ref.attr('class', 'action-enable');
            ref.attr('title', '<?php echo lang('users_index_thead_tip_inactive');?>');
            ref.children(":first").attr('class', 'mdi mdi-account mdi-dark mdi-inactive');
        });
    });

    //Delete a user
    $("#action-delete").on('click',  function(){
        var id = $(this).data('id');
        $.post( "<?php echo base_url();?>users/account", { operation: "delete", id: id }).done(function() {
            oTable.rows('tr[data-id="' + id + '"]').remove().draw();
            $('#frmConfirmDelete').modal('hide');
        });
    });