mirror of
https://github.com/InvoiceShelf/InvoiceShelf.git
synced 2026-07-18 06:45:20 +00:00
fix(security): recompute document totals server-side (GHSA-8c69) (#665)
Invoice/estimate/recurring-invoice creation and update accepted total, sub_total, tax and due_amount straight from the request with no server-side recalculation, so a client could persist financial totals that don't match the line items (and, on invoice update, corrupt the due-amount/paid-amount logic which keyed off the client total). - Adds App\Support\DocumentTotals which recomputes item totals (round(price * quantity) minus per-item discount) and document totals (sub_total, tax with per-item vs document and tax_included handling, total, due_amount), mirroring the front-end calculation. Only price/quantity/discounts/tax-line amounts are trusted. - getInvoicePayload/getEstimatePayload/getRecurringInvoicePayload override the client totals with the recomputed values; createItems recomputes each stored item total; Invoice::updateInvoice keys its due-amount/paid-amount logic off the recomputed total instead of the request 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:
committed by
GitHub
parent
d615cfb4ff
commit
107a951126
@@ -8,6 +8,7 @@ use App\Facades\PDF;
|
||||
use App\Mail\SendInvoiceMail;
|
||||
use App\Services\SerialNumberFormatter;
|
||||
use App\Space\PdfTemplateUtils;
|
||||
use App\Support\DocumentTotals;
|
||||
use App\Support\SafeOrderBy;
|
||||
use App\Traits\GeneratesPdfTrait;
|
||||
use App\Traits\HasCustomFieldsTrait;
|
||||
@@ -392,12 +393,12 @@ class Invoice extends Model implements HasMedia
|
||||
return 'customer_cannot_be_changed_after_payment_is_added';
|
||||
}
|
||||
|
||||
if ($request->total >= 0 && $request->total < $total_paid_amount) {
|
||||
if ($data['total'] >= 0 && $data['total'] < $total_paid_amount) {
|
||||
return 'total_invoice_amount_must_be_more_than_paid_amount';
|
||||
}
|
||||
|
||||
if ($oldTotal != $request->total) {
|
||||
$oldTotal = (int) round($request->total) - (int) $oldTotal;
|
||||
if ($oldTotal != $data['total']) {
|
||||
$oldTotal = (int) round($data['total']) - (int) $oldTotal;
|
||||
} else {
|
||||
$oldTotal = 0;
|
||||
}
|
||||
@@ -506,6 +507,9 @@ class Invoice extends Model implements HasMedia
|
||||
foreach ($invoiceItems as $invoiceItem) {
|
||||
$invoiceItem['company_id'] = $invoice->company_id;
|
||||
$invoiceItem['exchange_rate'] = $exchange_rate;
|
||||
// Recompute the item total from price/quantity so a tampered item
|
||||
// total can't desync from the recomputed document totals (GHSA-8c69).
|
||||
$invoiceItem['total'] = DocumentTotals::itemTotal($invoiceItem, $invoice->discount_per_item === 'YES');
|
||||
$invoiceItem['base_price'] = $invoiceItem['price'] * $exchange_rate;
|
||||
$invoiceItem['base_discount_val'] = $invoiceItem['discount_val'] * $exchange_rate;
|
||||
$invoiceItem['base_tax'] = $invoiceItem['tax'] * $exchange_rate;
|
||||
|
||||
Reference in New Issue
Block a user