[Robustness] Fundraiser RecordSale QuantitySold has no upper bound — absurd values can overflow accumulated totals #695

New Issue

2026-07-17T23:50:58Z

spikerj commented

2026-07-17 23:50:58 +00:00

Finding (input validation / data integrity — low severity)

RecordSaleCommandValidator requires QuantitySold > 0 (good — blocks negative/zero manipulation), but sets no upper bound. RecordSaleCommandHandler does item.QuantitySold += request.QuantitySold where item.QuantitySold is an int. A caller (the fundraiser's own owner — ownership is enforced) could record an absurd quantity near int.MaxValue; a second such sale overflows the accumulated int, wrapping it negative and corrupting the fundraiser's TotalRaisedCents.

Scope is limited: ownership is checked (can only affect one's OWN fundraiser), pricing is server-side (SellingPriceCents from the item, not the client — no price manipulation), and the money math itself is overflow-safe (SellingPriceCents is long, so long * int is computed in long). So this is a data-integrity robustness gap, not a security exploit — but absurd quantities shouldn't be accepted.

Fix (this PR)

Add QuantitySold.LessThanOrEqualTo(1_000_000) to the validator (a single sale recording of >1M units is not a legitimate input; incremental recording is expected). Prevents the int overflow / absurd totals. Added a validator test for the new bound.

## Finding (input validation / data integrity — low severity) `RecordSaleCommandValidator` requires `QuantitySold > 0` (good — blocks negative/zero manipulation), but sets **no upper bound**. `RecordSaleCommandHandler` does `item.QuantitySold += request.QuantitySold` where `item.QuantitySold` is an `int`. A caller (the fundraiser's own owner — ownership is enforced) could record an absurd quantity near `int.MaxValue`; a second such sale overflows the accumulated `int`, wrapping it negative and corrupting the fundraiser's `TotalRaisedCents`. Scope is limited: ownership is checked (can only affect one's OWN fundraiser), pricing is server-side (`SellingPriceCents` from the item, not the client — no price manipulation), and the money math itself is overflow-safe (`SellingPriceCents` is `long`, so `long * int` is computed in long). So this is a data-integrity robustness gap, not a security exploit — but absurd quantities shouldn't be accepted. ## Fix (this PR) Add `QuantitySold.LessThanOrEqualTo(1_000_000)` to the validator (a single sale recording of >1M units is not a legitimate input; incremental recording is expected). Prevents the int overflow / absurd totals. Added a validator test for the new bound.

spikerj commented

2026-07-18 00:02:55 +00:00

Resolved in spikersoft-backend PR #390 (merged to master). Capped RecordSale QuantitySold at 1,000,000 in the validator so absurd values can't overflow the accumulated int total. 7 validator tests pass. Closing.

Resolved in spikersoft-backend PR #390 (merged to `master`). Capped RecordSale QuantitySold at 1,000,000 in the validator so absurd values can't overflow the accumulated int total. 7 validator tests pass. Closing.

spikerj closed this issue

2026-07-18 00:02:55 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#695