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)
This commit is contained in:
Ollama
2026-03-24 19:03:27 +00:00
parent eec270d1aa
commit 34986c8e4e
2 changed files with 40 additions and 0 deletions

View File

@@ -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 {

View File

@@ -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",