Skip to content

Fix unbounded early-termination refund (security)#19

Merged
md5sha256 merged 1 commit into
mainfrom
develop
Jun 21, 2026
Merged

Fix unbounded early-termination refund (security)#19
md5sha256 merged 1 commit into
mainfrom
develop

Conversation

@ParadauxIO

Copy link
Copy Markdown
Contributor

Security fix β€” early lease termination refund could drain landlords

The bug

RealtyBackendImpl.unrentRegion computed the pro-rata refund as:

double refund = price * remainingSeconds / totalSeconds;

where remainingSeconds is now β†’ endDate but totalSeconds is a single durationSeconds. renewLeasehold (MariaLeaseholdContractMapper) extends endDate by another durationSeconds per renewal without re-escrowing, and renewals are unlimited when maxExtensions is null. So a tenant could:

  1. Rent a region for price,
  2. Renew it N times (free),
  3. unrent β†’ refund price Γ— N, debited from the landlord.

The ledger stays balanced (no minting), but it drains the landlord's real balance.

Observed in production (DemocracyCraft, 2026-06-14): a $150 rent refunded as $49,999.73; ~$294k moved from three landlords to two tenant accounts in ~20 minutes.

The fix

Clamp remainingSeconds to a single period (totalSeconds), so the refund can never exceed one period's price β€” the amount actually paid for the current term.

long remainingSeconds = Math.min(rawRemainingSeconds, totalSeconds);

Test

Adds a regression test (RealtyBackendImplTest.UnrentRefund): rent β†’ renew Γ—5 β†’ unrent must refund ≀ one period's price. Pre-fix this returned price Γ— 6.

The realty-backend test suite is Testcontainers-based (needs Docker); it was compile-verified locally and runs fully in CI. The fix + test compile clean.

Follow-ups (not in this PR β€” separate decisions)

  • Decide whether renewals should be charged, or whether free extension periods should count toward the refund at all.
  • Enforce a sane default maxExtensions (no unlimited renewals).

Remediation (out of band)

The exploited balances still need reversing (claw back the two tenant accounts; restore the three landlords). Staff issued a partial fine already.

unrentRegion computed the pro-rata refund as
    price * remainingSeconds / totalSeconds
where remainingSeconds came from (now -> endDate) but totalSeconds was a single
durationSeconds. renewLeasehold pushes endDate out by another durationSeconds per
renewal without re-escrowing, so a tenant could rent a region, renew it N times
(free, and unlimited when maxExtensions is null), then unrent for a refund of
price * N β€” draining the landlord's account. Observed in production: a $150 rent
refunded as $49,999.73.

Clamp remainingSeconds to a single period (totalSeconds) so the refund can never
exceed one period's price β€” the amount actually paid for the current term.

Adds a regression test (rent -> renew x5 -> unrent) asserting refund <= price.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@md5sha256 md5sha256 merged commit f6846ba into main Jun 21, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants