bug: validate wallet top-up amount and card before crediting balance - #340
devin-ai-integration[bot] wants to merge 1 commit into
Conversation
Signed-off-by: Devin AI <devin-ai-integration[bot]@users.noreply.github.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if (!Number.isInteger(amount) || amount < MIN_TOP_UP_AMOUNT || amount > MAX_TOP_UP_AMOUNT) { | ||
| return null |
There was a problem hiding this comment.
🟡 Fractional wallet deposits fail after card selection
When balanceControl accepts a fractional amount, parseTopUpAmount rejects it after card selection. Users traverse the payment flow before learning that the accepted deposit cannot complete.
Learn more
The wallet form defines only required, minimum, and maximum validators. Angular therefore treats values such as 10.5 as valid and enables the continue button. The payment page converts the stored value with parseFloat and submits it through choosePayment. The new API integer check then returns 400, despite both client screens allowing the user to proceed.
Example: A user enters 10.5. The deposit button enables, card selection succeeds, and the final request returns “Top-up amount must be a whole number” instead of adding the displayed amount.
Recommended fix: Add an integer validator to balanceControl and display its validation error before navigation. Keep the frontend and API bounds and integer contract synchronized.
Was this helpful? React with 👍 or 👎 to provide feedback.
| try { | ||
| await WalletModel.increment({ balance: req.body.balance }, { where: { UserId: req.body.UserId } }) | ||
| res.status(200).json({ status: 'success', data: req.body.balance }) | ||
| await WalletModel.increment({ balance: amount }, { where: { UserId: req.body.UserId } }) |
There was a problem hiding this comment.
🟨 Repeated top-ups bypass the credit cap
A card owner can repeat valid addWalletBalance requests without limit. Each request credits uncharged funds, so the wallet remains unbounded.
Was this helpful? React with 👍 or 👎 to provide feedback.
Description
Fixes a payment bypass in
PUT /rest/wallet/balance(routes/wallet.tsaddWalletBalance): the handler credited the wallet by the raw client-suppliedreq.body.balanceafter only checking that the caller owned some saved card. Since saved cards are never charged and the wallet balance is accepted as payment at checkout (routes/order.ts,paymentId === 'wallet'), any customer could grant themselves unlimited store credit.Changes:
parseTopUpAmount()— the amount must be an integer in[10, 1000](the same bounds the wallet UI already enforces client-side); anything else (negative, huge, fractional, non-numeric, missing) →400. The server-parsedamountis what is credited and echoed back, neverreq.body.balance.paymentIdis coerced to a positive integer before the card lookup; an expired card (expYear/expMonthin the past) is rejected with402like a foreign/missing card.test/api/wallet.test.tsfor negative, over-max (and balance unchanged), non-numeric and fractional amounts.Juice Shop has no real payment processor, so a genuine charge cannot be recorded; this fix bounds the exposure to the per-request limits the product already advertises. Seed cards all have expiry years ≥ 2081 so existing tests/e2e flows are unaffected.
Resolved or fixed issue: none
AI Tool Disclosure
DevinDevin (Cognition AI)Fix code-scan finding: payment bypass via unvalidated req.body.balance in routes/wallet.ts addWalletBalanceAffirmation
Devin-Org: engineering