From 72f147074da7316629e797119636abf5143e59fb Mon Sep 17 00:00:00 2001 From: jekkos Date: Sun, 22 Sep 2024 23:57:29 +0200 Subject: [PATCH] Enable html escape + fix XSS (#3965) --- app/Controllers/Customers.php | 29 ++------------------- app/Controllers/Items.php | 46 ++-------------------------------- app/Helpers/tabular_helper.php | 15 ++++------- public/js/manage_tables.js | 4 +-- 4 files changed, 11 insertions(+), 83 deletions(-) diff --git a/app/Controllers/Customers.php b/app/Controllers/Customers.php index a3e76d762..9778a605a 100644 --- a/app/Controllers/Customers.php +++ b/app/Controllers/Customers.php @@ -114,7 +114,7 @@ class Customers extends Persons $stats->quantity = 0; } - $data_rows[] = get_customer_data_row($person, $stats); //TODO: We either need to create a function to sanitize $person here (and for line 77) or we need to sanitize inside of get_customer_data_row(). + $data_rows[] = get_customer_data_row($person, $stats); } echo json_encode (['total' => $total_rows, 'rows' => $data_rows]); @@ -253,9 +253,7 @@ class Customers extends Persons } } - $sanitized_data = $this->sanitizeCustomerData($data); - - echo view("customers/form", $sanitized_data); + echo view("customers/form", $data); } /** @@ -535,27 +533,4 @@ class Customers extends Persons } } - /** - * Sanitizes customer values to remove unsafe HTML tags and javascript. - * This is not meant to replace CI4 sanitization. - * - * @param array $data Attribute data to sanitize. - * @return array Sanitized Attribute data. - */ - private function sanitizeCustomerData(array $data): array - { - $data['person_info']->first_name = Services::htmlPurifier()->purify($data['person_info']->first_name); - $data['person_info']->last_name = Services::htmlPurifier()->purify($data['person_info']->last_name); - $data['person_info']->address_1 = Services::htmlPurifier()->purify($data['person_info']->address_1); - $data['person_info']->address_2 = Services::htmlPurifier()->purify($data['person_info']->address_2); - $data['person_info']->city = Services::htmlPurifier()->purify($data['person_info']->city); - $data['person_info']->state = Services::htmlPurifier()->purify($data['person_info']->state); - $data['person_info']->zip = Services::htmlPurifier()->purify($data['person_info']->zip); - $data['person_info']->country = Services::htmlPurifier()->purify($data['person_info']->country); - $data['person_info']->comments = Services::htmlPurifier()->purify($data['person_info']->comments); - - $data['person_info']->company_name = Services::htmlPurifier()->purify($data['person_info']->company_name); - - return $data; - } } diff --git a/app/Controllers/Items.php b/app/Controllers/Items.php index ab2fc255b..2fdd0fd43 100644 --- a/app/Controllers/Items.php +++ b/app/Controllers/Items.php @@ -419,9 +419,7 @@ class Items extends Secure_Controller $data['selected_low_sell_item'] = ''; } - $sanitized_data = $this->sanitizeItemData($data); - - echo view('items/form', $sanitized_data); + echo view('items/form', $data); } /** @@ -548,9 +546,7 @@ class Items extends Secure_Controller unset($data['definition_names'][$definition_id]); } - $sanitized_data = $this->sanitizeAttributeData($data); - - echo view('attributes/item', $sanitized_data); + echo view('attributes/item', $data); } /** @@ -1443,42 +1439,4 @@ class Items extends Secure_Controller } } } - - /** - * Sanitizes unsafe data prior to sending it to the view. - * This is not meant to replace CI4 sanitization. - * - * @param array $data - * @return array - */ - private function sanitizeItemData(array $data): array - { - $data['item_info']->name = Services::htmlPurifier()->purify($data['item_info']->name); - $data['item_info']->category = Services::htmlPurifier()->purify($data['item_info']->category); - $data['item_info']->item_number = Services::htmlPurifier()->purify($data['item_info']->item_number); - $data['item_info']->description = Services::htmlPurifier()->purify($data['item_info']->description); - - return $data; - } - - /** - * Sanitizes TEXT type attribute values to remove unsafe HTML tags and javascript. - * Table data is not sanitized here. - * This is not meant to replace CI4 sanitization. - * - * @param array $data Attribute data to sanitize. - * @return array Sanitized Attribute data. - */ - private function sanitizeAttributeData(array $data): array - { - foreach($data['definition_values'] as $definition_id => &$definition_values) - { - if($definition_values['definition_type'] === 'TEXT') - { - $definition_values['attribute_value']->attribute_value = Services::htmlPurifier()->purify($definition_values['attribute_value']->attribute_value); - } - } - - return $data; - } } diff --git a/app/Helpers/tabular_helper.php b/app/Helpers/tabular_helper.php index 333a0e385..722e963e6 100644 --- a/app/Helpers/tabular_helper.php +++ b/app/Helpers/tabular_helper.php @@ -282,7 +282,7 @@ function get_customer_data_row(object $person, object $stats): array return [ 'people.person_id' => $person->person_id, 'last_name' => $person->last_name, - 'first_name' => Services::htmlPurifier()->purify($person->first_name), + 'first_name' => $person->first_name, 'email' => empty($person->email) ? '' : mailto($person->email, $person->email), 'phone_number' => $person->phone_number, 'total' => to_currency($stats->total), @@ -480,10 +480,10 @@ function get_item_data_row(object $item): array $columns = [ 'items.item_id' => $item->item_id, - 'item_number' => Services::htmlPurifier()->purify($item->item_number), - 'name' => Services::htmlPurifier()->purify($item->name), - 'category' => Services::htmlPurifier()->purify($item->category), - 'company_name' => Services::htmlPurifier()->purify($item->company_name), //TODO: This isn't in the items table. Should this be here? + 'item_number' => $item->item_number, + 'name' => $item->name, + 'category' => $item->category, + 'company_name' => $item->company_name, //TODO: This isn't in the items table. Should this be here? 'cost_price' => to_currency($item->cost_price), 'unit_price' => to_currency($item->unit_price), 'quantity' => to_quantity_decimals($item->quantity), @@ -651,11 +651,6 @@ function expand_attribute_values(array $definition_names, array $row): array if(isset($indexed_values[$definition_id])) { $attribute_value = $indexed_values[$definition_id]; - if(is_string($attribute_value)) - { - $attribute_value = Services::htmlPurifier()->purify($attribute_value); - } - $attribute_values["$definition_id"] = $attribute_value; } else diff --git a/public/js/manage_tables.js b/public/js/manage_tables.js index e985cbd26..f192f1c6a 100644 --- a/public/js/manage_tables.js +++ b/public/js/manage_tables.js @@ -258,7 +258,7 @@ iconSize: 'sm', silentSort: true, paginationVAlign: 'bottom', - escape: false + escape: true })); enable_actions(); init_delete(); @@ -287,7 +287,7 @@ return function (resource, response) { var id = response.id !== undefined ? response.id.toString() : ""; if (!response.success) { - $.notify(response.message, { type: 'danger' }); + $.notify($.text(response.message).html(), { type: 'danger' }); } else { var message = response.message; var selector = rows_selector(response.id);