XSS mitigation features (#4041)

* Remove HtmlPurifier calls

- All calls to Services::htmlPurifier()->purify() removed from data received from view.
- Bootstrap and bootswatch bump in package-lock.json

Signed-off-by: objecttothis <objecttothis@gmail.com>

* Pre-view filtering Items Controller

- Refactored code for clarity
- Created and called sanitization functions.
- Sanitize TEXT type Attributes before being sent to the view.

Signed-off-by: objecttothis <objecttothis@gmail.com>

* Pre-view filtering Customers Controller

- Refactored code for clarity
- Replaced == with === operator to prevent type juggling
- Added Sanitization of Customer data before being sent to the view

Signed-off-by: objecttothis <objecttothis@gmail.com>

* Bump bootstrap-table to 1.23.1

- Bump bootstrap-table to 1.23.1 in attempt to resolve issue with sticky headers
- Sanitize attribute data in tables
- Sanitize item data with controller function.

Signed-off-by: objecttothis <objecttothis@gmail.com>

* Pre-view filtering Items Controller

- Refactored code for clarity
- Created and called sanitization functions.
- Sanitize TEXT type Attributes before being sent to the view.

Signed-off-by: objecttothis <objecttothis@gmail.com>

* Sanitize Item data

- Sanitize category and item_number before display in forms.
- refactor check in pic_filename for empty to be best practices compliant.
- Added TODO

Signed-off-by: objecttothis <objecttothis@gmail.com>

* Minor changes

- Refactored for code clarity.
- Removed extra blank lines.
- Minor reformatting.
- Added PHPdocs
- bumped bootstrap-table to 1.23.2

Signed-off-by: objecttothis <objecttothis@gmail.com>

* Pre-view filtering Items Controller

- Refactored code for clarity
- Created and called sanitization functions.
- Sanitize TEXT type Attributes before being sent to the view.

Signed-off-by: objecttothis <objecttothis@gmail.com>

* Sanitize Item data

- Sanitize category and item_number before display in forms.
- refactor check in pic_filename for empty to be best practices compliant.
- Added TODO

Signed-off-by: objecttothis <objecttothis@gmail.com>

---------

Signed-off-by: objecttothis <objecttothis@gmail.com>
Co-authored-by: objecttothis <objecttothis@gmail.com>
This commit is contained in:
jekkos
2024-08-26 09:35:56 +02:00
committed by GitHub
parent 402997f0da
commit f49d763254
25 changed files with 158 additions and 88 deletions

View File

@@ -38,7 +38,7 @@ class Attributes extends Secure_Controller
*/
public function getSearch(): void
{
$search = Services::htmlPurifier()->purify($this->request->getGet('search'));
$search = $this->request->getGet('search');
$limit = $this->request->getGet('limit', FILTER_SANITIZE_NUMBER_INT);
$offset = $this->request->getGet('offset', FILTER_SANITIZE_NUMBER_INT);
$sort = $this->request->getGet('sort', FILTER_SANITIZE_FULL_SPECIAL_CHARS);

View File

@@ -43,7 +43,7 @@ class Cashups extends Secure_Controller
*/
public function getSearch(): void
{
$search = Services::htmlPurifier()->purify($this->request->getGet('search'));
$search = $this->request->getGet('search');
$limit = $this->request->getGet('limit', FILTER_SANITIZE_NUMBER_INT);
$offset = $this->request->getGet('offset', FILTER_SANITIZE_NUMBER_INT);
$sort = $this->request->getGet('sort', FILTER_SANITIZE_FULL_SPECIAL_CHARS);

View File

@@ -87,7 +87,7 @@ class Customers extends Persons
*/
public function getSearch(): void
{
$search = Services::htmlPurifier()->purify($this->request->getGet('search'));
$search = $this->request->getGet('search');
$limit = $this->request->getGet('limit', FILTER_SANITIZE_NUMBER_INT);
$offset = $this->request->getGet('offset', FILTER_SANITIZE_NUMBER_INT);
$sort = $this->request->getGet('sort', FILTER_SANITIZE_FULL_SPECIAL_CHARS);
@@ -114,7 +114,7 @@ class Customers extends Persons
$stats->quantity = 0;
}
$data_rows[] = get_customer_data_row($person, $stats);
$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().
}
echo json_encode (['total' => $total_rows, 'rows' => $data_rows]);
@@ -125,7 +125,7 @@ class Customers extends Persons
*/
public function getSuggest(): void
{
$search = Services::htmlPurifier()->purify($this->request->getGet('term'));
$search = $this->request->getPost('term');
$suggestions = $this->customer->get_search_suggestions($search);
echo json_encode($suggestions);
@@ -136,7 +136,7 @@ class Customers extends Persons
*/
public function suggest_search(): void
{
$search = Services::htmlPurifier()->purify($this->request->getPost('term'));
$search = $this->request->getPost('term');
$suggestions = $this->customer->get_search_suggestions($search, 25, false);
echo json_encode($suggestions);
@@ -185,14 +185,7 @@ class Customers extends Persons
$data['packages'] = $packages;
$data['selected_package'] = $info->package_id;
if($this->config['use_destination_based_tax']) //TODO: This can be shortened for ternary notation
{
$data['use_destination_based_tax'] = true;
}
else
{
$data['use_destination_based_tax'] = false;
}
$data['use_destination_based_tax'] = $this->config['use_destination_based_tax'];
// retrieve the total amount the customer spent so far together with min, max and average values
$stats = $this->customer->get_stats($customer_id);
@@ -260,7 +253,9 @@ class Customers extends Persons
}
}
echo view("customers/form", $data);
$sanitized_data = $this->sanitizeCustomerData($data);
echo view("customers/form", $sanitized_data);
}
/**
@@ -539,4 +534,28 @@ 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;
}
}

View File

@@ -27,7 +27,7 @@ class Employees extends Persons
*/
public function getSearch(): void
{
$search = Services::htmlPurifier()->purify($this->request->getGet('search'));
$search = $this->request->getGet('search');
$limit = $this->request->getGet('limit', FILTER_SANITIZE_NUMBER_INT);
$offset = $this->request->getGet('offset', FILTER_SANITIZE_NUMBER_INT);
$sort = $this->request->getGet('sort', FILTER_SANITIZE_FULL_SPECIAL_CHARS);
@@ -52,7 +52,7 @@ class Employees extends Persons
*/
public function getSuggest(): void
{
$search = Services::htmlPurifier()->purify($this->request->getPost('term'));
$search = $this->request->getPost('term');
$suggestions = $this->employee->get_search_suggestions($search, 25, true);
echo json_encode($suggestions);
@@ -63,7 +63,7 @@ class Employees extends Persons
*/
public function suggest_search(): void
{
$search = Services::htmlPurifier()->purify($this->request->getPost('term'));
$search = $this->request->getPost('term');
$suggestions = $this->employee->get_search_suggestions($search);
echo json_encode($suggestions);

View File

@@ -45,7 +45,7 @@ class Expenses extends Secure_Controller
*/
public function getSearch(): void
{
$search = Services::htmlPurifier()->purify($this->request->getGet('search'));
$search = $this->request->getGet('search');
$limit = $this->request->getGet('limit', FILTER_SANITIZE_NUMBER_INT);
$offset = $this->request->getGet('offset', FILTER_SANITIZE_NUMBER_INT);
$sort = $this->request->getGet('sort', FILTER_SANITIZE_FULL_SPECIAL_CHARS);

View File

@@ -31,7 +31,7 @@ class Expenses_categories extends Secure_Controller //TODO: Is this class ever u
**/
public function getSearch(): void
{
$search = Services::htmlPurifier()->purify($this->request->getGet('search'));
$search = $this->request->getGet('search');
$limit = $this->request->getGet('limit', FILTER_SANITIZE_NUMBER_INT);
$offset = $this->request->getGet('offset', FILTER_SANITIZE_NUMBER_INT);
$sort = $this->request->getGet('sort', FILTER_SANITIZE_FULL_SPECIAL_CHARS);

View File

@@ -32,7 +32,7 @@ class Giftcards extends Secure_Controller
*/
public function getSearch(): void
{
$search = Services::htmlPurifier()->purify($this->request->getGet('search'));
$search = $this->request->getGet('search');
$limit = $this->request->getGet('limit', FILTER_SANITIZE_NUMBER_INT);
$offset = $this->request->getGet('offset', FILTER_SANITIZE_NUMBER_INT);
$sort = $this->request->getGet('sort', FILTER_SANITIZE_FULL_SPECIAL_CHARS);
@@ -58,7 +58,7 @@ class Giftcards extends Secure_Controller
*/
public function getSuggest(): void
{
$search = Services::htmlPurifier()->purify($this->request->getPost('term'));
$search = $this->request->getPost('term');
$suggestions = $this->giftcard->get_search_suggestions($search, true);
echo json_encode($suggestions);
@@ -69,7 +69,7 @@ class Giftcards extends Secure_Controller
*/
public function suggest_search(): void
{
$search = Services::htmlPurifier()->purify($this->request->getPost('term'));
$search = $this->request->getPost('term');
$suggestions = $this->giftcard->get_search_suggestions($search);
echo json_encode($suggestions);

View File

@@ -76,7 +76,7 @@ class Item_kits extends Secure_Controller
*/
public function getSearch(): void
{
$search = Services::htmlPurifier()->purify($this->request->getGet('search')) ?? '';
$search = $this->request->getGet('search') ?? '';
$limit = $this->request->getGet('limit', FILTER_SANITIZE_NUMBER_INT);
$offset = $this->request->getGet('offset', FILTER_SANITIZE_NUMBER_INT);
$sort = $this->request->getGet('sort', FILTER_SANITIZE_FULL_SPECIAL_CHARS);
@@ -101,7 +101,7 @@ class Item_kits extends Secure_Controller
*/
public function suggest_search(): void
{
$search = Services::htmlPurifier()->purify($this->request->getPost('term'));
$search = $this->request->getPost('term');
$suggestions = $this->item_kit->get_search_suggestions($search);
echo json_encode($suggestions);

View File

@@ -94,7 +94,7 @@ class Items extends Secure_Controller
**/
public function getSearch(): void
{
$search = Services::htmlPurifier()->purify($this->request->getGet('search'));
$search = $this->request->getGet('search');
$limit = $this->request->getGet('limit', FILTER_SANITIZE_NUMBER_INT);
$offset = $this->request->getGet('offset', FILTER_SANITIZE_NUMBER_INT);
$sort = $this->request->getGet('sort', FILTER_SANITIZE_FULL_SPECIAL_CHARS);
@@ -182,7 +182,7 @@ class Items extends Secure_Controller
'is_deleted' => $this->request->getPost('is_deleted') !== null
];
$search = Services::htmlPurifier()->purify($this->request->getPost('term'));
$search = $this->request->getPost('term');
$suggestions = $this->item->get_search_suggestions($search, $options);
echo json_encode($suggestions);
@@ -195,7 +195,7 @@ class Items extends Secure_Controller
*/
public function getSuggest(): void
{
$search = Services::htmlPurifier()->purify($this->request->getPost('term'));
$search = $this->request->getPost('term');
$suggestions = $this->item->get_search_suggestions($search, ['search_custom' => false, 'is_deleted' => false], true);
echo json_encode($suggestions);
@@ -267,7 +267,7 @@ class Items extends Secure_Controller
* @param int $item_id
* @return void
*/
public function getView(int $item_id = NEW_ENTRY): void //TODO: Super long function. Perhaps we need to refactor out some methods.
public function getView(int $item_id = NEW_ENTRY): void //TODO: Long function. Perhaps we need to refactor out some methods.
{
// Set default values
if($item_id == null) $item_id = NEW_ENTRY;
@@ -277,8 +277,7 @@ class Items extends Secure_Controller
$data = [];
}
//allow_temp_items is set in the index function of items.php or sales.php
$data['allow_temp_item'] = $this->session->get('allow_temp_items');
$data['allow_temp_item'] = $this->session->get('allow_temp_items'); //allow_temp_items is set in the index function of items.php or sales.php
$data['item_tax_info'] = $this->item_taxes->get_info($item_id);
$data['default_tax_1_rate'] = '';
$data['default_tax_2_rate'] = '';
@@ -316,7 +315,7 @@ class Items extends Secure_Controller
$item_info->receiving_quantity = 1;
$item_info->reorder_level = 1;
$item_info->item_type = ITEM; //Standard
$item_info->item_type = ITEM; //Standard
$item_info->item_id = $item_id;
$item_info->stock_type = HAS_STOCK;
$item_info->tax_category_id = null;
@@ -328,7 +327,6 @@ class Items extends Secure_Controller
$item_info->tax_category_id = $this->config['default_tax_category'];
}
}
$data['standard_item_locked'] = (
$data['item_kit_disabled']
&& $item_info->item_type == ITEM_KIT
@@ -348,14 +346,9 @@ class Items extends Secure_Controller
$data['suppliers'] = $suppliers;
$data['selected_supplier'] = $item_info->supplier_id;
if($data['include_hsn']) //TODO: Transform this to ternary notation
{
$data['hsn_code'] = $item_info->hsn_code;
}
else
{
$data['hsn_code'] = '';
}
$data['hsn_code'] = $data['include_hsn']
? $item_info->hsn_code
: '';
if($use_destination_based_tax)
{
@@ -428,7 +421,9 @@ class Items extends Secure_Controller
$data['selected_low_sell_item'] = '';
}
echo view('items/form', $data);
$sanitized_data = $this->sanitizeItemData($data);
echo view('items/form', $sanitized_data);
}
/**
@@ -555,7 +550,9 @@ class Items extends Secure_Controller
unset($data['definition_names'][$definition_id]);
}
echo view('attributes/item', $data);
$sanitized_data = $this->sanitizeAttributeData($data);
echo view('attributes/item', $sanitized_data);
}
/**
@@ -1443,4 +1440,40 @@ 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']->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.
* 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;
}
}

View File

@@ -35,7 +35,7 @@ abstract class Persons extends Secure_Controller
*/
public function getSuggest(): void
{
$search = Services::htmlPurifier()->purify($this->request->getPost('term'));
$search = $this->request->getPost('term');
$suggestions = $this->person->get_search_suggestions($search);
echo json_encode($suggestions);

View File

@@ -61,7 +61,7 @@ class Receivings extends Secure_Controller
*/
public function getItemSearch(): void
{
$search = Services::htmlPurifier()->purify($this->request->getGet('term'));
$search = $this->request->getGet('term');
$suggestions = $this->item->get_search_suggestions($search, ['search_custom' => false, 'is_deleted' => false], true);
$suggestions = array_merge($suggestions, $this->item_kit->get_search_suggestions($search));
@@ -76,7 +76,7 @@ class Receivings extends Secure_Controller
*/
public function getStockItemSearch(): void
{
$search = Services::htmlPurifier()->purify($this->request->getGet('term'));
$search = $this->request->getGet('term');
$suggestions = $this->item->get_stock_search_suggestions($search, ['search_custom' => false, 'is_deleted' => false], true);
$suggestions = array_merge($suggestions, $this->item_kit->get_search_suggestions($search));

View File

@@ -186,7 +186,7 @@ class Sales extends Secure_Controller
{
$suggestions = [];
$receipt = $search = $this->request->getGet('term') != ''
? Services::htmlPurifier()->purify($this->request->getGet('term'))
? $this->request->getGet('term')
: null;
if($this->sale_lib->get_mode() == 'return' && $this->sale->is_valid_receipt($receipt))
@@ -206,7 +206,7 @@ class Sales extends Secure_Controller
public function suggest_search(): void
{
$search = $this->request->getPost('term') != ''
? Services::htmlPurifier()->purify($this->request->getPost('term'))
? $this->request->getPost('term')
: null;
$suggestions = $this->sale->get_search_suggestions($search);

View File

@@ -45,7 +45,7 @@ class Suppliers extends Persons
**/
public function getSearch(): void
{
$search = Services::htmlPurifier()->purify($this->request->getGet('search'));
$search = $this->request->getGet('search');
$limit = $this->request->getGet('limit', FILTER_SANITIZE_NUMBER_INT);
$offset = $this->request->getGet('offset', FILTER_SANITIZE_NUMBER_INT);
$sort = $this->request->getGet('sort', FILTER_SANITIZE_FULL_SPECIAL_CHARS);
@@ -71,7 +71,7 @@ class Suppliers extends Persons
**/
public function getSuggest(): void
{
$search = Services::htmlPurifier()->purify($this->request->getGet('term'));
$search = $this->request->getGet('term');
$suggestions = $this->supplier->get_search_suggestions($search, true);
echo json_encode($suggestions);
@@ -82,7 +82,7 @@ class Suppliers extends Persons
*/
public function suggest_search(): void
{
$search = Services::htmlPurifier()->purify($this->request->getPost('term'));
$search = $this->request->getPost('term');
$suggestions = $this->supplier->get_search_suggestions($search, false);
echo json_encode($suggestions);

View File

@@ -36,7 +36,7 @@ class Tax_categories extends Secure_Controller
*/
public function getSearch(): void
{
$search = Services::htmlPurifier()->purify($this->request->getGet('search'));
$search = $this->request->getGet('search');
$limit = $this->request->getGet('limit', FILTER_SANITIZE_NUMBER_INT);
$offset = $this->request->getGet('offset', FILTER_SANITIZE_NUMBER_INT);
$sort = $this->request->getGet('sort', FILTER_SANITIZE_FULL_SPECIAL_CHARS);

View File

@@ -46,7 +46,7 @@ class Tax_codes extends Secure_Controller
*/
public function getSearch(): void
{
$search = Services::htmlPurifier()->purify($this->request->getGet('search'));
$search = $this->request->getGet('search');
$limit = $this->request->getGet('limit', FILTER_SANITIZE_NUMBER_INT);
$offset = $this->request->getGet('offset', FILTER_SANITIZE_NUMBER_INT);
$sort = $this->request->getGet('sort', FILTER_SANITIZE_FULL_SPECIAL_CHARS);

View File

@@ -39,7 +39,7 @@ class Tax_jurisdictions extends Secure_Controller
*/
public function getSearch(): void
{
$search = Services::htmlPurifier()->purify($this->request->getGet('search'));
$search = $this->request->getGet('search');
$limit = $this->request->getGet('limit', FILTER_SANITIZE_NUMBER_INT);
$offset = $this->request->getGet('offset', FILTER_SANITIZE_NUMBER_INT);
$sort = $this->request->getGet('sort', FILTER_SANITIZE_FULL_SPECIAL_CHARS);

View File

@@ -83,7 +83,7 @@ class Taxes extends Secure_Controller
*/
public function getSearch(): void
{
$search = Services::htmlPurifier()->purify($this->request->getGet('search'));
$search = $this->request->getGet('search');
$limit = $this->request->getGet('limit', FILTER_SANITIZE_NUMBER_INT);
$offset = $this->request->getGet('offset', FILTER_SANITIZE_NUMBER_INT);
$sort = $this->request->getGet('sort', FILTER_SANITIZE_FULL_SPECIAL_CHARS);
@@ -107,7 +107,7 @@ class Taxes extends Secure_Controller
*/
public function suggest_search(): void
{
$search = Services::htmlPurifier()->purify($this->request->getPost('term'));
$search = $this->request->getPost('term');
$suggestions = $this->tax->get_search_suggestions($search); //TODO: There is no get_search_suggestions function in the tax model
echo json_encode($suggestions);
@@ -120,7 +120,7 @@ class Taxes extends Secure_Controller
*/
public function suggest_tax_categories(): void
{
$search = Services::htmlPurifier()->purify($this->request->getPost('term'));
$search = $this->request->getPost('term');
$suggestions = $this->tax_category->get_tax_category_suggestions($search);
echo json_encode($suggestions);
@@ -461,7 +461,7 @@ class Taxes extends Secure_Controller
*/
public function getSuggestTaxCodes(): void
{
$search = Services::htmlPurifier()->purify($this->request->getPostGet('term'));
$search = $this->request->getPostGet('term');
$suggestions = $this->tax_code->get_tax_codes_search_suggestions($search);
echo json_encode($suggestions);