mirror of
https://github.com/opensourcepos/opensourcepos.git
synced 2026-05-16 20:38:32 -04:00
fix(security): SQL injection and path traversal vulnerabilities (#4539)
Security fixes for two vulnerabilities:
1. SQL Injection in Summary Sales Taxes Report (GHSA-5j9m-2f98-cjqw)
- Fixed unsanitized user input concatenation in getData() method
- Applied proper escaping using $this->db->escape() for start_date/end_date
- Consistent with existing _where() method implementation
2. Path Traversal in Receipt Template (GHSA-h6wm-fhw2-m3q3)
- Added ALLOWED_RECEIPT_TEMPLATES whitelist constant
- Added isValidReceiptTemplate() validation method
- Validate receipt_template before saving in Config controller
- Validate receipt_template before rendering in receipt view
- Default to 'receipt_default' for invalid values
- Consistent with invoice_type fix pattern (commit 31d25e06d)
Affected files:
- app/Models/Reports/Summary_sales_taxes.php
- app/Libraries/Sale_lib.php
- app/Controllers/Config.php
- app/Views/sales/receipt.php
Co-authored-by: Ollama <ollama@steganos.dev>
This commit is contained in:
@@ -924,7 +924,9 @@ class Config extends Secure_Controller
|
||||
public function postSaveReceipt(): ResponseInterface
|
||||
{
|
||||
$batch_save_data = [
|
||||
'receipt_template' => $this->request->getPost('receipt_template'),
|
||||
'receipt_template' => Sale_lib::isValidReceiptTemplate($this->request->getPost('receipt_template'))
|
||||
? $this->request->getPost('receipt_template')
|
||||
: 'receipt_default',
|
||||
'receipt_font_size' => $this->request->getPost('receipt_font_size', FILTER_SANITIZE_NUMBER_INT),
|
||||
'print_delay_autoreturn' => $this->request->getPost('print_delay_autoreturn', FILTER_SANITIZE_NUMBER_INT),
|
||||
'email_receipt_check_behaviour' => $this->request->getPost('email_receipt_check_behaviour'),
|
||||
|
||||
@@ -908,6 +908,14 @@ class Sales extends Secure_Controller
|
||||
return $this->_reload($data);
|
||||
} else {
|
||||
$data['barcode'] = $this->barcode_lib->generate_receipt_barcode($data['sale_id']);
|
||||
|
||||
// Validate receipt template to prevent path traversal
|
||||
$receipt_template = $this->config['receipt_template'] ?? '';
|
||||
if (!Sale_lib::isValidReceiptTemplate($receipt_template)) {
|
||||
$receipt_template = 'receipt_default';
|
||||
}
|
||||
$data['receipt_template_view'] = $receipt_template;
|
||||
|
||||
$this->sale_lib->clear_all();
|
||||
return view('sales/receipt', $data);
|
||||
}
|
||||
@@ -1163,6 +1171,13 @@ class Sales extends Secure_Controller
|
||||
}
|
||||
$data['invoice_view'] = $invoice_type;
|
||||
|
||||
// Validate receipt template to prevent path traversal
|
||||
$receipt_template = $this->config['receipt_template'] ?? '';
|
||||
if (!Sale_lib::isValidReceiptTemplate($receipt_template)) {
|
||||
$receipt_template = 'receipt_default';
|
||||
}
|
||||
$data['receipt_template_view'] = $receipt_template;
|
||||
|
||||
return $data;
|
||||
}
|
||||
|
||||
|
||||
@@ -108,6 +108,11 @@ class Sale_lib
|
||||
'custom_tax_invoice'
|
||||
];
|
||||
|
||||
private const ALLOWED_RECEIPT_TEMPLATES = [
|
||||
'receipt_default',
|
||||
'receipt_short'
|
||||
];
|
||||
|
||||
public function get_invoice_type_options(): array
|
||||
{
|
||||
$invoice_types = [];
|
||||
@@ -161,6 +166,11 @@ class Sale_lib
|
||||
return in_array($invoice_type, self::ALLOWED_INVOICE_TYPES, true);
|
||||
}
|
||||
|
||||
public static function isValidReceiptTemplate(string $receipt_template): bool
|
||||
{
|
||||
return in_array($receipt_template, self::ALLOWED_RECEIPT_TEMPLATES, true);
|
||||
}
|
||||
|
||||
/**
|
||||
* @return array
|
||||
*/
|
||||
|
||||
@@ -33,14 +33,16 @@ class Summary_sales_taxes extends Summary_report
|
||||
* @param object $builder
|
||||
* @return void
|
||||
*/
|
||||
protected function _where(array $inputs, object &$builder): void // TODO: hungarian notation
|
||||
protected function _where(array $inputs, object &$builder): void
|
||||
{
|
||||
$builder->where('sales.sale_status', COMPLETED);
|
||||
|
||||
if (empty($this->config['date_or_time_format'])) { // TODO: Duplicated code
|
||||
$builder->where('DATE(sales.sale_time) BETWEEN ' . $this->db->escape($inputs['start_date']) . ' AND ' . $this->db->escape($inputs['end_date']));
|
||||
if (empty($this->config['date_or_time_format'])) {
|
||||
$builder->where('DATE(sales.sale_time) >=', $inputs['start_date']);
|
||||
$builder->where('DATE(sales.sale_time) <=', $inputs['end_date']);
|
||||
} else {
|
||||
$builder->where('sales.sale_time BETWEEN ' . $this->db->escape(rawurldecode($inputs['start_date'])) . ' AND ' . $this->db->escape(rawurldecode($inputs['end_date'])));
|
||||
$builder->where('sales.sale_time >=', $inputs['start_date']);
|
||||
$builder->where('sales.sale_time <=', $inputs['end_date']);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -53,9 +55,11 @@ class Summary_sales_taxes extends Summary_report
|
||||
$builder = $this->db->table('sales_taxes');
|
||||
|
||||
if (empty($this->config['date_or_time_format'])) {
|
||||
$builder->where('DATE(sale_time) BETWEEN ' . $inputs['start_date'] . ' AND ' . $inputs['end_date']);
|
||||
$builder->where('DATE(sale_time) >=', $inputs['start_date']);
|
||||
$builder->where('DATE(sale_time) <=', $inputs['end_date']);
|
||||
} else {
|
||||
$builder->where('sale_time BETWEEN ' . $this->db->escape(rawurldecode($inputs['start_date'])) . ' AND ' . $this->db->escape(rawurldecode($inputs['end_date'])));
|
||||
$builder->where('sale_time >=', $inputs['start_date']);
|
||||
$builder->where('sale_time <=', $inputs['end_date']);
|
||||
}
|
||||
|
||||
$builder->select('reporting_authority, jurisdiction_name, tax_category, tax_rate, SUM(sale_tax_amount) AS tax');
|
||||
|
||||
@@ -2,11 +2,14 @@
|
||||
/**
|
||||
* @var int $sale_id_num
|
||||
* @var bool $print_after_sale
|
||||
* @var string $receipt_template_view
|
||||
* @var array $config
|
||||
*/
|
||||
|
||||
use App\Models\Employee;
|
||||
|
||||
$template = $receipt_template_view ?? 'receipt_default';
|
||||
|
||||
?>
|
||||
|
||||
<?= view('partial/header') ?>
|
||||
@@ -61,6 +64,6 @@ if (isset($error_message)) {
|
||||
<?php endif; ?>
|
||||
</div>
|
||||
|
||||
<?= view('sales/' . $config['receipt_template']) ?>
|
||||
<?= view('sales/' . $template) ?>
|
||||
|
||||
<?= view('partial/footer') ?>
|
||||
|
||||
Reference in New Issue
Block a user