From 34986c8e4e1fa36378980314c6c81331c179f1a5 Mon Sep 17 00:00:00 2001 From: Ollama Date: Tue, 24 Mar 2026 19:03:27 +0000 Subject: [PATCH] Fix business logic vulnerability allowing negative sale totals An authenticated employee with sales permission could: - Create negative-total sales (store "pays" the customer) - Set discounts > 100% for negative-total effect - Bypass inventory controls with negative quantities This fix adds validation in: - postEditItem(): validates discount <= 100% for percentage discounts, discount <= item total for fixed discounts, and non-negative price/quantity/discount - postComplete(): blocks sale completion if total is negative (exceptions for returns) CVSS v3.1: 6.5 Medium (AV:N/AC:L/PR:L/UI:N/S:U/C:N/I:H/A:N) --- app/Controllers/Sales.php | 34 ++++++++++++++++++++++++++++++++++ app/Language/en/Sales.php | 6 ++++++ 2 files changed, 40 insertions(+) diff --git a/app/Controllers/Sales.php b/app/Controllers/Sales.php index 628ae5057..57eb8dd5a 100644 --- a/app/Controllers/Sales.php +++ b/app/Controllers/Sales.php @@ -597,6 +597,34 @@ class Sales extends Secure_Controller ? parse_quantity($this->request->getPost('discount')) : parse_decimals($this->request->getPost('discount')); + if ($price < 0) { + $data['error'] = lang('Sales.negative_price_invalid'); + return $this->_reload($data); + } + + if ($quantity < 0) { + $data['error'] = lang('Sales.negative_quantity_invalid'); + return $this->_reload($data); + } + + if ($discount < 0) { + $data['error'] = lang('Sales.negative_discount_invalid'); + return $this->_reload($data); + } + + if ($discount_type == PERCENT && $discount > 100) { + $data['error'] = lang('Sales.discount_percent_exceeds_100'); + return $this->_reload($data); + } + + if ($discount_type == FIXED) { + $item_total = bcmul($quantity, $price); + if ($discount > $item_total) { + $data['error'] = lang('Sales.discount_exceeds_item_total'); + return $this->_reload($data); + } + } + $item_location = $this->request->getPost('location', FILTER_SANITIZE_NUMBER_INT); $discounted_total = $this->request->getPost('discounted_total') != '' ? parse_decimals($this->request->getPost('discounted_total') ?? '') @@ -723,6 +751,12 @@ class Sales extends Secure_Controller $data['cash_amount_due'] = $totals['cash_amount_due']; $data['non_cash_amount_due'] = $totals['amount_due']; + // Prevent negative total sales (fraud/theft vector) - returns can have negative totals for legitimate refunds + if ($this->sale_lib->get_mode() != 'return' && bccomp($totals['total'], '0') < 0) { + $data['error'] = lang('Sales.negative_total_invalid'); + return $this->_reload($data); + } + if ($data['cash_mode']) { // TODO: Convert this to ternary notation $data['amount_due'] = $totals['cash_amount_due']; } else { diff --git a/app/Language/en/Sales.php b/app/Language/en/Sales.php index 6c7d93d48..eedfe8ee7 100644 --- a/app/Language/en/Sales.php +++ b/app/Language/en/Sales.php @@ -73,6 +73,12 @@ return [ "employee" => "Employee", "entry" => "Entry", "error_editing_item" => "Error editing item", + "negative_discount_invalid" => "Discount cannot be negative.", + "negative_price_invalid" => "Price cannot be negative.", + "negative_quantity_invalid" => "Quantity cannot be negative.", + "discount_percent_exceeds_100" => "Percentage discount cannot exceed 100%.", + "discount_exceeds_item_total" => "Discount cannot exceed the item total.", + "negative_total_invalid" => "Sale total cannot be negative. Check item discounts and quantities.", "find_or_scan_item" => "Find or Scan Item", "find_or_scan_item_or_receipt" => "Find or Scan Item or Receipt", "giftcard" => "Gift Card",