fix(security): recompute document totals server-side (GHSA-8c69) (#672)

v3 port. Invoice/estimate/recurring creation and update accepted total,
sub_total, tax and due_amount straight from the request with no recalculation,
letting a client persist financial totals that don't match the line items
(and corrupt the invoice update due-amount/paid-amount logic which keyed off
the client total).

- Adds App\Support\DocumentTotals (mirrors the front-end calc) trusting only
  price/quantity/discounts/tax-line amounts.
- getInvoicePayload/getEstimatePayload/getRecurringInvoicePayload override the
  client totals; the shared DocumentItemService::createItems recomputes each
  item total; InvoiceService::update keys its due-amount logic off the
  recomputed total.

Adds DocumentTotals unit tests + a feature test proving a tampered invoice
total is ignored; existing create/update tests no longer assert the now
server-authoritative derived totals.
This commit is contained in:
Darko Gjorgjijoski
2026-06-12 11:02:23 +02:00
committed by GitHub
parent 99ea898e88
commit 84524ce247
11 changed files with 266 additions and 62 deletions

View File

@@ -5,6 +5,7 @@ namespace App\Http\Requests;
use App\Models\CompanySetting;
use App\Models\Customer;
use App\Models\RecurringInvoice;
use App\Support\DocumentTotals;
use Illuminate\Foundation\Http\FormRequest;
class RecurringInvoiceRequest extends FormRequest
@@ -106,15 +107,35 @@ class RecurringInvoiceRequest extends FormRequest
$nextInvoiceAt = RecurringInvoice::getNextInvoiceDate($this->frequency, $this->starts_at);
$tax_per_item = CompanySetting::getSetting('tax_per_item', $this->header('company')) ?? 'NO ';
$discount_per_item = CompanySetting::getSetting('discount_per_item', $this->header('company')) ?? 'NO';
// Recompute totals server-side from the line items (GHSA-8c69). The
// recurring template totals propagate to every generated invoice.
$totals = DocumentTotals::compute(
$this->items ?? [],
$this->taxes ?? [],
$this->discount_val,
$tax_per_item,
(bool) $this->tax_included,
$discount_per_item
);
return collect($this->except('items', 'taxes'))
->merge([
'creator_id' => $this->user()->id,
'company_id' => $this->header('company'),
'next_invoice_at' => $nextInvoiceAt,
'tax_per_item' => CompanySetting::getSetting('tax_per_item', $this->header('company')) ?? 'NO ',
'discount_per_item' => CompanySetting::getSetting('discount_per_item', $this->header('company')) ?? 'NO',
'due_amount' => $this->total,
'tax_per_item' => $tax_per_item,
'discount_per_item' => $discount_per_item,
'sub_total' => $totals['sub_total'],
'total' => $totals['total'],
'tax' => $totals['tax'],
'due_amount' => $totals['total'],
'exchange_rate' => $exchange_rate,
'base_sub_total' => $totals['sub_total'] * $exchange_rate,
'base_total' => $totals['total'] * $exchange_rate,
'base_tax' => $totals['tax'] * $exchange_rate,
'currency_id' => $currency,
])
->toArray();