# Payment integrity: `amount_to_pay` is client-asserted, not reconciled **Severity: Critical — open.** Requires the payments team; do not fix without understanding the installment/proration rules below, they're easy to break. **Status:** the *discount* half of this was hardened in `OnlinePaymentCollectionService` (see "What's already fixed"). The *core* issue — the fee amount itself — is still exploitable and is the reason this stays Critical. ## The bug, in one sentence `OnlinePaymentCollectionService::processFeeVersionPayments()` decides how much an invoice item costs, and therefore whether it's `status: 'completed'`, from `amount_to_pay` values the **client sent in the payment request** — never from `StudentFeePlanVersionDetail.monthly_amount` (the real, admin-set fee) or from what has actually been paid on that item before. ## Where (all in `app/Services/OnlinePaymentCollectionService.php`) ```php // line 347 $amountToPay = $amounts['fee_amount'] + $amounts['fine_amount']; // <- client's own claim // line 349-353 $feeDetail = StudentFeePlanVersionDetail::find($invoiceItemId); ... $monthlyAmount = $feeDetail->monthly_amount ?? 0; // <- the REAL fee. Fetched... // line 465 $paymentForThisItem = min($feeVersionPaymentTotal, $amountToPay); // ...and $monthlyAmount is // never referenced again. // line 486 $lastAmount = $amountToPay - $paymentForThisItem; // line 480-497 $invoiceItem = InvoiceItem::updateOrCreate([...], [ 'actual_amount' => $amounts['fee_amount'], // client value, stored as truth 'paid_amount' => (...) + $paymentForThisItem, 'last_amount' => $lastAmount, 'status' => $lastAmount <= 0 ? 'completed' : 'partial', ... ]); ``` `$amounts['fee_amount']` / `$amounts['fine_amount']` trace back to `fee_versions[].invoice_items[].amount_to_pay` in the request body accepted by `GeniePaymentController::create()` (`app/Http/Controllers/GeniePaymentController.php:42`) and `MyFeesPaymentController::create()`. The order's `amount` (what the payment gateway is asked to collect) is summed from the **same client-supplied** `amount_to_pay` values at create time — there's no point in the flow where the requested amount is checked against what the item actually costs. ## Exploit 1. `POST /api/v1/payments/genie/create` with a real, owned `invoice_item_id` whose actual `monthly_amount` is e.g. 10,000, but `amount_to_pay: 1`. 2. Pay the resulting LKR 1 order for real through Genie's checkout. 3. Genie's webhook confirms `SUCCESS` for amount `1` — which matches `$order->amount` (also 1, because it was computed from the same client claim) — so the round-15 amount-mismatch guard does **not** trigger; it only compares the webhook amount to the order amount, and both were forged together at step 1. 4. `processPayment()` runs; `$lastAmount = 1 - 1 = 0` → the invoice item is stored as `status: 'completed'`. ## Confirmed impact (not cosmetic) `PaymentController` computes "what's overdue / still owed" by explicitly skipping settled items: ```php // app/Http/Controllers/Api/V1/Admin/PaymentController.php:225-231 if (($fee['is_paid'] ?? false) === true) { continue; } ... if ($paymentStatus === 'paid') { continue; } ``` A falsely-`completed` item **disappears from every outstanding-balance and overdue view** — there is no independent ledger that would still flag it. Anything gated on "no outstanding fees" (LMS access, exam admission, ID card issuance, etc.) is almost certainly bypassed the same way. ## What's already fixed (round 19, same file) The discount side of this had the identical shape of bug and is now closed: `calculateInvoiceItemDiscounts()` (the authoritative, DB-backed discount calculator, driven by real `DiscountAssign` rows) used to be invoked only when the client's `discount_amount` / `applied_discounts` looked "inconsistent." A self-consistent *fake* discount sailed through. It's now called unconditionally and the accepted discount is capped at the real entitlement. That fix does **not** touch `amount_to_pay` / `fee_amount` — a student doesn't even need to fake a discount to exploit this; a tiny `amount_to_pay` alone is enough. ## Suggested fix shape (needs your domain sign-off, not a drop-in) Recompute the item's true remaining balance server-side instead of trusting the request: ``` outstanding = monthly_amount // for a single-installment item, or the // correct installment share for multi- // installment plans — see the existing // per-installment math in // PaymentController.php:244-260 as the // reference for how installments/last- // installment remainder are computed - existing_paid_amount // InvoiceItem.paid_amount before this txn - authoritative_discount // now correctly capped (round 19) accepted_payment = min(client_offered_amount, outstanding) // partial payment is fine new_last_amount = outstanding - accepted_payment status = new_last_amount <= 0 ? 'completed' : 'partial' ``` Open questions for whoever picks this up (this is exactly why it wasn't attempted blind): - Which installment does a given `invoice_item_id` map to, and how does the last-installment remainder (`PaymentController.php:255`, `max(0, $netAmount - ($monthlyAmount * ($installmentCount - 1)))`) apply here vs. in the read path? - Is `direct_total_discount_allocated` (also client-supplied today, line 491) a legitimate separate allocation that needs its own authoritative source, or should it be folded into the same entitlement check? - Are there legitimate flows where `amount_to_pay` is intentionally less than `monthly_amount` for reasons other than "the student is choosing to pay part now" (e.g. staff-granted one-off adjustments) that must still be possible after this is locked down? ## Also open, smaller - `create()` has no check that the caller owns the `invoice_item_id`s in the request (any authenticated user could build an order against another student's fee items — though without also fixing the amount trust above, the practical impact is limited to creating orders, not diverting payment credit). - `direct_total_discount_allocated` is a second client-supplied discount-like field with no authoritative check (see above).