From a22c0cb149ca3ce1cf1409da8812e8f487ce5ae5 Mon Sep 17 00:00:00 2001 From: Peter Goodhall Date: Sat, 5 Oct 2019 19:35:55 +0100 Subject: [PATCH] Assorted Security improvements --- application/controllers/Awards.php | 8 ++++++++ application/controllers/Backup.php | 7 +++++++ application/controllers/Lookup.php | 9 +++++++++ application/controllers/Notes.php | 9 +++++++++ application/controllers/User.php | 10 +++++----- application/models/User_model.php | 28 ++++++++++++++++++++-------- 6 files changed, 58 insertions(+), 13 deletions(-) diff --git a/application/controllers/Awards.php b/application/controllers/Awards.php index 2495ba92..5490b62b 100644 --- a/application/controllers/Awards.php +++ b/application/controllers/Awards.php @@ -7,6 +7,14 @@ */ class Awards extends CI_Controller { + + function __construct() + { + parent::__construct(); + + $this->load->model('user_model'); + if(!$this->user_model->authorize(2)) { $this->session->set_flashdata('notice', 'You\'re not allowed to do that!'); redirect('dashboard'); } + } public function index() { diff --git a/application/controllers/Backup.php b/application/controllers/Backup.php index cae40257..b1bb3a5b 100644 --- a/application/controllers/Backup.php +++ b/application/controllers/Backup.php @@ -1,7 +1,14 @@ load->model('user_model'); + if(!$this->user_model->authorize(2)) { $this->session->set_flashdata('notice', 'You\'re not allowed to do that!'); redirect('dashboard'); } + } + /* User Facing Links to Backup URLs */ public function index() { diff --git a/application/controllers/Lookup.php b/application/controllers/Lookup.php index c44c31da..03e54c9e 100644 --- a/application/controllers/Lookup.php +++ b/application/controllers/Lookup.php @@ -8,6 +8,15 @@ class Lookup extends CI_Controller { + + function __construct() + { + parent::__construct(); + + $this->load->model('user_model'); + if(!$this->user_model->authorize(2)) { $this->session->set_flashdata('notice', 'You\'re not allowed to do that!'); redirect('dashboard'); } + } + public function index() { diff --git a/application/controllers/Notes.php b/application/controllers/Notes.php index f4356398..a8de4a47 100644 --- a/application/controllers/Notes.php +++ b/application/controllers/Notes.php @@ -2,6 +2,15 @@ class Notes extends CI_Controller { + function __construct() + { + parent::__construct(); + + $this->load->model('user_model'); + if(!$this->user_model->authorize(2)) { $this->session->set_flashdata('notice', 'You\'re not allowed to do that!'); redirect('dashboard'); } + } + + /* Displays all notes in a list */ public function index() { diff --git a/application/controllers/User.php b/application/controllers/User.php index 090f4943..e2912196 100644 --- a/application/controllers/User.php +++ b/application/controllers/User.php @@ -236,21 +236,21 @@ class User extends CI_Controller { switch($this->user_model->edit($this->input->post())) { // Check for errors case EUSERNAMEEXISTS: - $data['username_error'] = 'Username '.$this->input->post('user_name').' already in use!'; + $data['username_error'] = 'Username '.$this->input->post('user_name', true).' already in use!'; break; case EEMAILEXISTS: - $data['email_error'] = 'E-mail address '.$this->input->post('user_email').' already in use!'; + $data['email_error'] = 'E-mail address '.$this->input->post('user_email', true).' already in use!'; break; case EPASSWORDINVALID: $data['password_error'] = 'Invalid password!'; break; // All okay, return to user screen case OK: - if($this->session->userdata('user_id') == $this->input->post('id')) { - $this->session->set_flashdata('notice', 'User '.$this->input->post('user_name').' edited'); + if($this->session->userdata('user_id') == $this->input->post('id', true)) { + $this->session->set_flashdata('notice', 'User '.$this->input->post('user_name', true).' edited'); redirect('user/profile'); } else { - $this->session->set_flashdata('notice', 'User '.$this->input->post('user_name').' edited'); + $this->session->set_flashdata('notice', 'User '.$this->input->post('user_name', true).' edited'); redirect('user'); } return; diff --git a/application/models/User_model.php b/application/models/User_model.php index 46dfb054..c8d6ab0d 100644 --- a/application/models/User_model.php +++ b/application/models/User_model.php @@ -22,15 +22,21 @@ class User_Model extends CI_Model { // FUNCTION: object get($username) // Retrieve a user function get($username) { - $this->db->where('user_name', $username); + // Clean ID + $clean_username = $this->security->xss_clean($username); + + $this->db->where('user_name', $clean_username); $r = $this->db->get($this->config->item('auth_table')); return $r; - } + } // FUNCTION: object get_by_id($id) // Retrieve a user by user ID function get_by_id($id) { - $this->db->where('user_id', $id); + // Clean ID + $clean_id = $this->security->xss_clean($id); + + $this->db->where('user_id', $clean_id); $r = $this->db->get($this->config->item('auth_table')); return $r; } @@ -38,7 +44,10 @@ class User_Model extends CI_Model { // FUNCTION: object get_by_email($email) // Retrieve a user by email address function get_by_email($email) { - $this->db->where('user_email', $email); + + $clean_email = $this->security->xss_clean($email); + + $this->db->where('user_email', $clean_email); $r = $this->db->get($this->config->item('auth_table')); return $r; } @@ -46,7 +55,8 @@ class User_Model extends CI_Model { // FUNCTION: bool exists($username) // Check if a user exists (by username) function exists($username) { - if($this->get($username)->num_rows() == 0) { + $clean_username = $this->security->xss_clean($username); + if($this->get($clean_username)->num_rows() == 0) { return 0; } else { return 1; @@ -56,7 +66,9 @@ class User_Model extends CI_Model { // FUNCTION: bool exists_by_id($id) // Check if a user exists (by user ID) function exists_by_id($id) { - if($this->get_by_id($id)->num_rows() == 0) { + $clean_id = $this->security->xss_clean($id); + + if($this->get_by_id($clean_id)->num_rows() == 0) { return 0; } else { return 1; @@ -196,8 +208,8 @@ class User_Model extends CI_Model { // This is really just a wrapper around User_Model::authenticate function login() { - $username = $this->input->post('user_name'); - $password = $this->input->post('user_password'); + $username = $this->input->post('user_name', true); + $password = $this->input->post('user_password', true); return $this->authenticate($username, $password); }