From 56c1b05fea1b77e3dbf6104a70ab8b3241b93a32 Mon Sep 17 00:00:00 2001 From: HB9HIL Date: Thu, 5 Sep 2024 23:59:16 +0200 Subject: [PATCH 1/5] added bindings to prevent SQL injection --- application/models/Oqrs_model.php | 121 ++++++++++++++++++------------ 1 file changed, 75 insertions(+), 46 deletions(-) diff --git a/application/models/Oqrs_model.php b/application/models/Oqrs_model.php index edad91b0a..bc2ae26f3 100644 --- a/application/models/Oqrs_model.php +++ b/application/models/Oqrs_model.php @@ -1,21 +1,23 @@ db->where('oqrs', "1"); return $this->db->get('station_profile'); } function get_station_info($station_id) { - $station_id = $this->security->xss_clean($station_id); + $binding = []; $sql = 'select count(*) as count, min(col_time_on) as mindate, max(col_time_on) as maxdate - from ' . $this->config->item('table_name') . ' where station_id = ' . $station_id; + from ' . $this->config->item('table_name') . ' where station_id = ?'; + $binding[] = $station_id; - $query = $this->db->query($sql); + $query = $this->db->query($sql, $binding); return $query->row(); } @@ -42,29 +44,22 @@ class Oqrs_model extends CI_Model { return $result; } - - function get_qsos_grouped($callsign){ - - // Populating array with worked band/mode combinations - $worked = $this->getQueryData($station_id, $callsign); - - $result['qsocount'] = count($worked); - $result['qsoarray'] = $resultArray; - - return $result; - } - /* * Builds query depending on what we are searching for */ function getQueryData($station_id, $callsign) { - $station_id = $this->security->xss_clean($station_id); - $callsign = $this->security->xss_clean($callsign); - $sql = 'select lower(col_mode) col_mode, coalesce(col_submode, "") col_submode, col_band from ' . $this->config->item('table_name') . ' where station_id = ' . $station_id . ' and col_call ="' . $callsign . '" and col_prop_mode != "SAT"'; - $sql .= ' union all select lower(col_mode) col_mode, coalesce(col_submode, "") col_submode, "SAT" col_band from ' . $this->config->item('table_name') . ' where station_id = ' . $station_id . ' and col_call ="' . $callsign . '" and col_prop_mode = "SAT"'; + $binding = []; - $query = $this->db->query($sql); + $sql = 'select lower(col_mode) col_mode, coalesce(col_submode, "") col_submode, col_band from ' . $this->config->item('table_name') . ' where station_id = ? and col_call = ? and col_prop_mode != "SAT"'; + $binding[] = $station_id; + $binding[] = $callsign; + + $sql .= ' union all select lower(col_mode) col_mode, coalesce(col_submode, "") col_submode, "SAT" col_band from ' . $this->config->item('table_name') . ' where station_id = ? and col_call = ? and col_prop_mode = "SAT"'; + $binding[] = $station_id; + $binding[] = $callsign; + + $query = $this->db->query($sql, $binding); return $query->result(); } @@ -73,14 +68,18 @@ class Oqrs_model extends CI_Model { * Builds query depending on what we are searching for */ function getQueryDataGrouped($callsign) { - $callsign = $this->security->xss_clean($callsign); - $sql = 'select lower(col_mode) col_mode, coalesce(col_submode, "") col_submode, col_band, station_callsign, station_profile_name, l.station_id from ' . $this->config->item('table_name') . ' as l join station_profile on l.station_id = station_profile.station_id where station_profile.oqrs = "1" and l.col_call ="' . $callsign . '" and l.col_prop_mode != "SAT"'; + + $binding = []; + + $sql = 'select lower(col_mode) col_mode, coalesce(col_submode, "") col_submode, col_band, station_callsign, station_profile_name, l.station_id from ' . $this->config->item('table_name') . ' as l join station_profile on l.station_id = station_profile.station_id where station_profile.oqrs = "1" and l.col_call = ? and l.col_prop_mode != "SAT"'; + $binding[] = $callsign; $sql .= ' union all select lower(col_mode) col_mode, coalesce(col_submode, "") col_submode, "SAT" col_band, station_callsign, station_profile_name, l.station_id from ' . $this->config->item('table_name') . ' l' . - ' join station_profile on l.station_id = station_profile.station_id where station_profile.oqrs = "1" and col_call ="' . $callsign . '" and col_prop_mode = "SAT"'; + ' join station_profile on l.station_id = station_profile.station_id where station_profile.oqrs = "1" and col_call = ? and col_prop_mode = "SAT"'; + $binding[] = $callsign; - $query = $this->db->query($sql); + $query = $this->db->query($sql, $binding); if ($query) { return $query->result(); @@ -190,9 +189,11 @@ class Oqrs_model extends CI_Model { } function delete_oqrs_line($id) { - $sql = 'delete from oqrs where id =' . xss_clean($id); + $binding = []; + $sql = 'delete from oqrs where id = ?'; + $binding[] = $id; - $query = $this->db->query($sql); + $query = $this->db->query($sql, $binding); return true; } @@ -224,16 +225,28 @@ class Oqrs_model extends CI_Model { } function check_oqrs($qsodata) { + + $binding = []; + $sql = 'select * from ' . $this->config->item('table_name') . - ' where (col_band = \'' . $qsodata['band'] . '\' or col_prop_mode = \'' . $qsodata['band'] . '\') - and col_call = \'' . $qsodata['requestcallsign'] . '\' - and date(col_time_on) = \'' . $qsodata['date'] . '\' - and (col_mode = \'' . $qsodata['mode'] . '\' - or col_submode = \'' . $qsodata['mode'] . '\') - and timediff(time(col_time_on), \'' . $qsodata['time'] . '\') <= 3000 - and station_id = ' . $qsodata['station_id']; + ' where (col_band = ? or col_prop_mode = ?) + and col_call = ? + and date(col_time_on) = ? + and (col_mode = ? + or col_submode = ?) + and timediff(time(col_time_on), ?) <= 3000 + and station_id = ?'; - $query = $this->db->query($sql); + $binding[] = $qsodata['band']; + $binding[] = $qsodata['band']; + $binding[] = $qsodata['requestcallsign']; + $binding[] = $qsodata['date']; + $binding[] = $qsodata['mode']; + $binding[] = $qsodata['mode']; + $binding[] = $qsodata['time']; + $binding[] = $qsodata['station_id']; + + $query = $this->db->query($sql, $binding); if ($result = $query->result()) { $id = 0; @@ -272,13 +285,23 @@ class Oqrs_model extends CI_Model { } function search_log_time_date($time, $date, $band, $mode) { + + $binding = []; + $sql = 'select * from ' . $this->config->item('table_name') . ' thcv - join station_profile on thcv.station_id = station_profile.station_id where (col_band = \'' . $band . '\' or col_prop_mode = \'' . $band . '\') - and date(col_time_on) = \'' . $date . '\' - and (col_mode = \'' . $mode . '\' - or col_submode = \'' . $mode . '\') - and timediff(time(col_time_on), \'' . $time . '\') <= 3000 - and station_profile.user_id = '. $this->session->userdata('user_id'); + join station_profile on thcv.station_id = station_profile.station_id where (col_band = ? or col_prop_mode = ?) + and date(col_time_on) = ? + and (col_mode = ? + or col_submode = ?) + and timediff(time(col_time_on), ?) <= 3000 + and station_profile.user_id = ?'; + $binding[] = $band; + $binding[] = $band; + $binding[] = $date; + $binding[] = $mode; + $binding[] = $mode; + $binding[] = $time; + $binding[] = $this->session->userdata('user_id'); return $this->db->query($sql);; } @@ -294,9 +317,11 @@ class Oqrs_model extends CI_Model { } function getQslInfo($station_id) { - $sql = 'select oqrs_text from station_profile where station_id = ' . $station_id; + $binding = []; + $sql = 'select oqrs_text from station_profile where station_id = ?'; + $binding[] = $station_id; - $query = $this->db->query($sql); + $query = $this->db->query($sql, $binding); if ($query->num_rows() > 0) { @@ -308,9 +333,11 @@ class Oqrs_model extends CI_Model { } function getOqrsEmailSetting($station_id) { - $sql = 'select oqrs_email from station_profile where station_id = ' . $station_id; + $binding = []; + $sql = 'select oqrs_email from station_profile where station_id = ?'; + $binding[] = $station_id; - $query = $this->db->query($sql); + $query = $this->db->query($sql, $binding); if ($query->num_rows() > 0) { @@ -387,9 +414,11 @@ class Oqrs_model extends CI_Model { } function getOqrsStationsFromSlug($logbook_id) { - $sql = 'SELECT station_callsign FROM `station_logbooks_relationship` JOIN `station_profile` ON station_logbooks_relationship.station_location_id = station_profile.station_id WHERE station_profile.oqrs = 1 AND station_logbook_id = '.$logbook_id.';'; + $binding = []; + $sql = 'SELECT station_callsign FROM `station_logbooks_relationship` JOIN `station_profile` ON station_logbooks_relationship.station_location_id = station_profile.station_id WHERE station_profile.oqrs = 1 AND station_logbook_id = ?;'; + $binding[] = $logbook_id; - $query = $this->db->query($sql); + $query = $this->db->query($sql, $binding); if ($query->num_rows() > 0) { return true; From 761f01ab3fc804eebde753d32bc24d28852496de Mon Sep 17 00:00:00 2001 From: HB9HIL Date: Fri, 6 Sep 2024 00:13:04 +0200 Subject: [PATCH 2/5] xss_clean ($this->input->post(key, TRUE)) --- application/controllers/Oqrs.php | 53 ++++++++++++++++---------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/application/controllers/Oqrs.php b/application/controllers/Oqrs.php index 6434b17b5..2f3c8a920 100644 --- a/application/controllers/Oqrs.php +++ b/application/controllers/Oqrs.php @@ -37,7 +37,7 @@ class Oqrs extends CI_Controller { $this->load->model('oqrs_model'); $this->load->model('publicsearch'); - $slug = $this->security->xss_clean($public_slug); + $slug = $this->security->xss_clean($public_slug); $data['slug'] = $slug; $data['oqrs_enabled'] = $this->oqrs_model->oqrs_enabled($slug); $data['public_search_enabled'] = $this->publicsearch->public_search_enabled($slug); @@ -54,7 +54,7 @@ class Oqrs extends CI_Controller { public function get_station_info() { $this->load->model('oqrs_model'); - $result = $this->oqrs_model->get_station_info($this->input->post('station_id')); + $result = $this->oqrs_model->get_station_info($this->input->post('station_id', TRUE)); header('Content-Type: application/json'); echo json_encode($result); @@ -62,11 +62,11 @@ class Oqrs extends CI_Controller { public function get_qsos() { $this->load->model('bands'); - $data['bands'] = $this->bands->get_worked_bands_oqrs($this->security->xss_clean($this->input->post('station_id'))); + $data['bands'] = $this->bands->get_worked_bands_oqrs($this->input->post('station_id', TRUE)); $this->load->model('oqrs_model'); - $result = $this->oqrs_model->get_qsos($this->input->post('station_id'), $this->input->post('callsign'), $data['bands']); - $data['callsign'] = $this->security->xss_clean($this->input->post('callsign')); + $result = $this->oqrs_model->get_qsos($this->input->post('station_id', TRUE), $this->input->post('callsign', TRUE), $data['bands']); + $data['callsign'] = $this->input->post('callsign', TRUE); $data['result'] = $result['qsoarray']; $data['qsocount'] = $result['qsocount']; @@ -75,8 +75,8 @@ class Oqrs extends CI_Controller { public function get_qsos_grouped() { $this->load->model('oqrs_model'); - $data['result'] = $this->oqrs_model->getQueryDataGrouped($this->input->post('callsign')); - $data['callsign'] = $this->security->xss_clean($this->input->post('callsign')); + $data['result'] = $this->oqrs_model->getQueryDataGrouped($this->input->post('callsign', TRUE)); + $data['callsign'] = $this->input->post('callsign', TRUE); if($this->input->post('widget') != 'true') { $this->load->view('oqrs/request_grouped', $data); @@ -97,7 +97,6 @@ class Oqrs extends CI_Controller { $data['page_title'] = __("Log Search & OQRS"); $this->load->model('bands'); - // $data['bands'] = $this->bands->get_worked_bands_oqrs($this->security->xss_clean($this->input->post('station_id'))); $this->load->view('oqrs/notinlogform', $data); } @@ -105,10 +104,10 @@ class Oqrs extends CI_Controller { public function save_not_in_log() { $station_ids = array(); - $postdata = $this->input->post(); + $postdata = $this->input->post(NULL, TRUE); // index is null means we get all postdata, TRUE means we XSS clean everything $this->load->model('oqrs_model'); $this->oqrs_model->save_not_in_log($postdata); - array_push($station_ids, xss_clean($this->input->post('station_id'))); + array_push($station_ids, $this->input->post('station_id', TRUE)); $this->alert_oqrs_request($postdata, $station_ids); } @@ -117,9 +116,9 @@ class Oqrs extends CI_Controller { */ public function request_form() { $this->load->model('oqrs_model'); - $data['result'] = $this->oqrs_model->getQueryData($this->input->post('station_id'), $this->input->post('callsign')); - $data['callsign'] = $this->security->xss_clean($this->input->post('callsign')); - $data['qslinfo'] = $this->oqrs_model->getQslInfo($this->input->post('station_id')); + $data['result'] = $this->oqrs_model->getQueryData($this->input->post('station_id', TRUE), $this->input->post('callsign', TRUE)); + $data['callsign'] = $this->input->post('callsign', TRUE); + $data['qslinfo'] = $this->oqrs_model->getQslInfo($this->input->post('station_id', TRUE)); $this->load->view('oqrs/request', $data); } @@ -146,40 +145,40 @@ class Oqrs extends CI_Controller { } public function save_oqrs_request() { - $postdata = $this->input->post(); + $postdata = $this->input->post(NULL, TRUE); // index is null means we get all postdata, TRUE means we XSS clean everything $this->load->model('oqrs_model'); $station_ids = $this->oqrs_model->save_oqrs_request($postdata); $this->alert_oqrs_request($postdata, $station_ids); } public function save_oqrs_request_grouped() { - $postdata = $this->input->post(); + $postdata = $this->input->post(NULL, TRUE); // index is null means we get all postdata, TRUE means we XSS clean everything $this->load->model('oqrs_model'); $station_ids = $this->oqrs_model->save_oqrs_request_grouped($postdata); $this->alert_oqrs_request($postdata, $station_ids); } public function delete_oqrs_line() { - $id = $this->input->post('id'); + $id = $this->input->post('id', TRUE); $this->load->model('oqrs_model'); $this->oqrs_model->delete_oqrs_line($id); } public function search_log() { $this->load->model('oqrs_model'); - $callsign = $this->input->post('callsign'); + $callsign = $this->input->post('callsign', TRUE); - $data['qsos'] = $this->oqrs_model->search_log($this->security->xss_clean($callsign)); + $data['qsos'] = $this->oqrs_model->search_log($callsign); $this->load->view('qslprint/qsolist', $data); } public function search_log_time_date() { $this->load->model('oqrs_model'); - $time = $this->security->xss_clean($this->input->post('time')); - $date = $this->security->xss_clean($this->input->post('date')); - $mode = $this->security->xss_clean($this->input->post('mode')); - $band = $this->security->xss_clean($this->input->post('band')); + $time = $this->input->post('time', TRUE); + $date = $this->input->post('date', TRUE); + $mode = $this->input->post('mode', TRUE); + $band = $this->input->post('band', TRUE); $data['qsos'] = $this->oqrs_model->search_log_time_date($time, $date, $band, $mode); @@ -238,7 +237,7 @@ class Oqrs extends CI_Controller { public function mark_oqrs_line_as_done() { $this->load->model('oqrs_model'); - $id = $this->security->xss_clean($this->input->post('id')); + $id = $this->input->post('id', TRUE); $this->oqrs_model->mark_oqrs_line_as_done($id); } @@ -248,10 +247,10 @@ class Oqrs extends CI_Controller { $searchCriteria = array( 'user_id' => (int)$this->session->userdata('user_id'), - 'de' => xss_clean($this->input->post('de')), - 'dx' => xss_clean($this->input->post('dx')), - 'status' => xss_clean($this->input->post('status')), - 'oqrsResults' => xss_clean($this->input->post('oqrsResults')), + 'de' => $this->input->post('de', TRUE), + 'dx' => $this->input->post('dx', TRUE), + 'status' => $this->input->post('status', TRUE), + 'oqrsResults' => $this->input->post('oqrsResults', TRUE), ); $qsos = $this->oqrs_model->searchOqrs($searchCriteria); From 5749a15924b69f1336be5f0ea1216cd1441df860 Mon Sep 17 00:00:00 2001 From: HB9HIL Date: Fri, 6 Sep 2024 00:13:25 +0200 Subject: [PATCH 3/5] we don't need to clean stuff twice, let's reduce some load --- application/models/Oqrs_model.php | 60 +++++++++++++++---------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/application/models/Oqrs_model.php b/application/models/Oqrs_model.php index bc2ae26f3..9441b3f79 100644 --- a/application/models/Oqrs_model.php +++ b/application/models/Oqrs_model.php @@ -127,15 +127,15 @@ class Oqrs_model extends CI_Model { $qsos = $postdata['qsos']; foreach($qsos as $qso) { $data = array( - 'date' => xss_clean($qso[0]), - 'time' => xss_clean($qso[1]), - 'band' => xss_clean($qso[2]), - 'mode' => xss_clean($qso[3]), - 'requestcallsign' => xss_clean($postdata['callsign']), - 'station_id' => xss_clean($postdata['station_id']), - 'note' => xss_clean($postdata['message']), - 'email' => xss_clean($postdata['email']), - 'qslroute' => xss_clean($postdata['qslroute']), + 'date' => $qso[0], + 'time' => $qso[1], + 'band' => $qso[2], + 'mode' => $qso[3], + 'requestcallsign' => $postdata['callsign'], + 'station_id' => $postdata['station_id'], + 'note' => $postdata['message'], + 'email' => $postdata['email'], + 'qslroute' => $postdata['qslroute'], 'status' => '0', ); @@ -147,8 +147,8 @@ class Oqrs_model extends CI_Model { $data['qsoid'] = $qsoid; $this->db->insert('oqrs', $data); - if(!in_array(xss_clean($postdata['station_id']), $station_ids)){ - array_push($station_ids, xss_clean($postdata['station_id'])); + if(!in_array($postdata['station_id'], $station_ids)){ + array_push($station_ids, $postdata['station_id']); } } @@ -160,15 +160,15 @@ class Oqrs_model extends CI_Model { $qsos = $postdata['qsos']; foreach($qsos as $qso) { $data = array( - 'date' => xss_clean($qso[0]), - 'time' => xss_clean($qso[1]), - 'band' => xss_clean($qso[2]), - 'mode' => xss_clean($qso[3]), - 'requestcallsign' => xss_clean($postdata['callsign']), - 'station_id' => xss_clean($qso[4]), - 'note' => xss_clean($postdata['message']), - 'email' => xss_clean($postdata['email']), - 'qslroute' => xss_clean($postdata['qslroute']), + 'date' => $qso[0], + 'time' => $qso[1], + 'band' => $qso[2], + 'mode' => $qso[3], + 'requestcallsign' => $postdata['callsign'], + 'station_id' => $qso[4], + 'note' => $postdata['message'], + 'email' => $postdata['email'], + 'qslroute' => $postdata['qslroute'], 'status' => '0', ); @@ -181,8 +181,8 @@ class Oqrs_model extends CI_Model { $this->db->insert('oqrs', $data); - if(!in_array(xss_clean($qso[4]), $station_ids)){ - array_push($station_ids, xss_clean($qso[4])); + if(!in_array($qso[4], $station_ids)){ + array_push($station_ids, $qso[4]); } } return $station_ids; @@ -207,14 +207,14 @@ class Oqrs_model extends CI_Model { $qsos = $postdata['qsos']; foreach($qsos as $qso) { $data = array( - 'date' => xss_clean($qso[0]), - 'time' => xss_clean($qso[1]), - 'band' => xss_clean($qso[2]), - 'mode' => xss_clean($qso[3]), - 'requestcallsign' => xss_clean($postdata['callsign']), - 'station_id' => xss_clean($postdata['station_id']), - 'note' => xss_clean($postdata['message']), - 'email' => xss_clean($postdata['email']), + 'date' => $qso[0], + 'time' => $qso[1], + 'band' => $qso[2], + 'mode' => $qso[3], + 'requestcallsign' => $postdata['callsign'], + 'station_id' => $postdata['station_id'], + 'note' => $postdata['message'], + 'email' => $postdata['email'], 'qslroute' => '', 'status' => '1', 'qsoid' => '0', From 20a48e31a4ab76a2ba650a3b1b6bfa221ef668fa Mon Sep 17 00:00:00 2001 From: HB9HIL Date: Fri, 6 Sep 2024 00:24:38 +0200 Subject: [PATCH 4/5] get rid of some console errors --- assets/js/sections/oqrs.js | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/assets/js/sections/oqrs.js b/assets/js/sections/oqrs.js index e061bdad7..279310854 100644 --- a/assets/js/sections/oqrs.js +++ b/assets/js/sections/oqrs.js @@ -49,7 +49,7 @@ function searchOqrsGrouped() { $.ajax({ url: base_url+'index.php/oqrs/get_qsos_grouped', type: 'post', - data: {'callsign': $("#oqrssearch").val().toUpperCase()}, + data: {'callsign': ($("#oqrssearch").val() || '').toUpperCase()}, success: function (data) { $(".searchinfo").append(data); $('.qsotime').change(function() { @@ -78,16 +78,19 @@ function searchOqrsGrouped() { // Get the input field var input = document.getElementById("emailInput"); - // Execute a function when the user presses a key on the keyboard - input.addEventListener("keypress", function(event) { - // If the user presses the "Enter" key on the keyboard - if (event.key === "Enter") { - // Cancel the default action, if needed - event.preventDefault(); - // Trigger the button element with a click - document.getElementById("requestGroupedSubmit").click(); + // If we are on the correct page we can add the EventListener + if (input) { + // Execute a function when the user presses a key on the keyboard + input.addEventListener("keypress", function(event) { + // If the user presses the "Enter" key on the keyboard + if (event.key === "Enter") { + // Cancel the default action, if needed + event.preventDefault(); + // Trigger the button element with a click + document.getElementById("requestGroupedSubmit").click(); + } + }); } - }); } }); } From f8df0c43368f49022b9e4889c0dc7a2ccd1aa9d0 Mon Sep 17 00:00:00 2001 From: HB9HIL Date: Fri, 6 Sep 2024 12:56:37 +0200 Subject: [PATCH 5/5] forgot one binding --- application/models/Oqrs_model.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application/models/Oqrs_model.php b/application/models/Oqrs_model.php index 9441b3f79..9177a46e4 100644 --- a/application/models/Oqrs_model.php +++ b/application/models/Oqrs_model.php @@ -303,7 +303,7 @@ class Oqrs_model extends CI_Model { $binding[] = $time; $binding[] = $this->session->userdata('user_id'); - return $this->db->query($sql);; + return $this->db->query($sql, $binding); } function mark_oqrs_line_as_done($id) {