Status: pre-submission review of the testnet demo. The review itself changed no contract. Afterwards the owner applied two of the cheap proposals before the final deploy:
- F-04 fixed:
PropertyFactoryno longer marks theRentDistributora trusted spender. The operator approves the distributor like any ERC-20 spender (the server does this once per property). - F-10 partly fixed:
setRegistrar,setTrustedSpenderandsetDistributornow emit events.Ownable2Stepwas not adopted: ownership hand-offs happen inside the factory in one transaction and a two-step flow would break that.
All other findings below remain open and are documented as known limitations of the testnet demo.
| Item | Detail |
|---|---|
| Contracts | KYCRegistry, MockIDR, PropertyToken, RentDistributor, PrimarySale, Marketplace, PropertyFactory (packages/foundry/contracts/) |
| Out of scope | Frontend, deploy scripts, offchain KYC, OpenZeppelin 5.x internals |
| Method | Manual read of all 7 contracts and the 26 existing tests, threat modelling (access control, reentrancy and ordering, rounding, KYC revocation, marketplace staleness, DoS), then executable evidence |
| Evidence | packages/foundry/test/Invariants.t.sol: 5 stateful invariants (256 runs, depth 50), 3 fuzz tests, 15 unit tests. Findings that could be demonstrated are pinned by test_KnownIssue_* tests that pass on the current code |
| Result | 45 tests pass (26 existing, 19 new). No loss of user funds or units was found that a non-operator can cause |
| Not done | No formal verification, no mainnet fork, no review of the deployed addresses, no gas profiling, no review of non-standard ERC-20 behaviour (the contracts only ever use MockIDR and PropertyToken) |
| Actor | Can | Cannot |
|---|---|---|
Operator (owner of KYCRegistry, MockIDR, PropertyFactory, and per property PropertyToken and RentDistributor) |
Verify or un-verify any wallet; mint unlimited tIDR; mark any address a trusted spender (spend any tIDR balance without approval, F-03); create properties; deposit rent; choose the treasury per property | Mint more units after mintInventory; re-set the distributor; change the sale price; withdraw unsold inventory; move anyone's units (units can only move by their holder or by an approved spender, and only between verified wallets) |
Registrar (the factory, in KYCRegistry and MockIDR) |
Same as the operator for setVerified and setTrustedSpender, but the factory exposes no function that uses this beyond createProperty |
Be called by anyone but its owner |
| Verified holder | Transfer units to any other verified wallet (including zero-value), list, claim | Send units to unverified wallets, claim another wallet's rent |
| Unverified wallet | Call createRoom; claim rent it already earned |
Receive units |
Test-only by design: MockIDR (free minting, trusted spender shortcut), the single-key operator, and the "KYC" flag (a boolean set by the operator, no real identity check).
All statuses are "not changed". "Verified" means shown by a passing test or by reading the code path.
| ID | Sev | Where | Description | Impact | Proposed fix | Evidence |
|---|---|---|---|---|---|---|
| F-01 | Low | RentDistributor._settle, PropertyToken._update |
_settle stores paid = accPerUnit after flooring balance * delta / 1e18, so the fractional part is lost each time a holder is settled. transfer(victim, 0) by any verified wallet settles the victim for free. With small deposits relative to supply (for example 10 tIDR over 110 units) a small holder who is settled after each deposit accrues 0 forever, while an identical un-settled holder accrues 18 |
User-exploitable griefing, loss is under 1 tIDR per grief and only for small holders; also occurs naturally when holders transfer often | Track remainders: keep accrued in 1e18 precision and divide only on claim, i.e. accruedScaled[h] += balance * (acc - paid[h]) and amount = accruedScaled[h] / 1e18 |
Verified: test_KnownIssue_ZeroTransferGriefsSmallHolderRent |
| F-02 | Medium (prod) / Info (demo) | KYCRegistry.setVerified, PropertyToken._update, RentDistributor.claim |
Un-verifying a holder freezes the units: they cannot be sent anywhere, not even back to the sale, and there is no operator recovery or forced transfer. Rent still accrues to the frozen units and the wallet can still claim it (claim has no KYC check). Lost keys are unrecoverable too | A revoked investor is locked in; operator can only re-verify. Not exploitable by users | Decide the policy and document it. Options: add forceTransfer(from, to, amount) owner-only with an event, or block claim for unverified wallets |
Verified: test_KnownIssue_RevokedHolderKeepsRentButUnitsAreFrozen |
| F-03 | Medium (operator only) | MockIDR.setTrustedSpender, allowance, _spendAllowance |
The owner or any registrar can mark any address a trusted spender, which then calls transferFrom(anyHolder, ...) with no approval. allowance() also reports max for every holder, which misleads wallets and indexers |
The operator can drain every tIDR balance. Test tokens only, but a judge may read it as a backdoor | Test token only: say so (see limitations). If kept on a public demo: restrict trusted spenders to a list frozen after deployment, or replace with permit / one-time max approval in the frontend |
Verified: test_KnownIssue_TrustedSpenderCanDrainAnyHolder |
| F-04 | Low | PropertyFactory.createProperty |
The factory also marks the RentDistributor a trusted spender, but the distributor only pulls from its owner in depositRent (the operator, who can simply approve). Extra privilege with no use |
Larger blast radius if the distributor were ever exploited | Delete idr.setTrustedSpender(address(distributor), true) |
Verified: test_KnownIssue_DistributorIsTrustedSpenderUnnecessarily |
| F-05 | Low | RentDistributor.depositRent |
Rent is a snapshot of balances at the deposit transaction. A verified wallet can buy units on the marketplace just before a deposit and sell them just after, taking a full period of rent for the spread (example test: sniper earns 1000 of 2000 while the long-term holder earns 0). The operator cannot prevent it | Economic unfairness for long-term holders. Needs a willing seller, so impact grows with marketplace liquidity | Operational: send depositRent privately or from a script that also pauses listings. Contract: accrue by time-weighted balance, or add a minimum holding period before rent accrues |
Verified: test_KnownIssue_RentSnipingRoundTrip |
| F-06 | Info | Marketplace.list, Marketplace.buy |
Listings are validated only at creation. A seller can list more units than they own across several listings, list without approval, or move units away. Fills then revert (InsufficientUnits or an allowance error). Price and units are immutable per listing, so a buyer cannot be switched to a worse price, and a failed fill costs only gas. Revoked sellers' listings remain visible |
UI shows unfillable listings | Frontend: filter by balanceOf and allowance before display. Optional contract: buy already re-checks balance, nothing to fix onchain |
Verified: test_KnownIssue_StaleAndOverlappingListings |
| F-07 | Info | PrimarySale.createRoom, buy |
Rooms are counters. Anyone (even unverified) can create a room, there is no cap, deadline, refund or recipient, and raisedUnits can exceed targetUnits. Titles are unbounded strings. Nobody holds money for a room |
Metrics can be gamed or spammed; the UI must render titles as plain text | Frontend: escape titles; optionally cap title length and reject buys beyond target | Verified: test_KnownIssue_RoomsAreCountersOnly, invariant RoomAccounting |
| F-08 | Info | PropertyToken._update |
A verified wallet can push units onto any other verified wallet without consent (also zero-value). This is the vector for F-01 | Unsolicited holdings; negligible | None required, or require opt-in | Verified: test_KnownIssue_UnsolicitedTransfersAreAllowed |
| F-09 | Info | RentDistributor |
Rounding dust stays in the contract and tIDR sent straight to it is unrecoverable; there is no sweep. Dust per deposit is under 1 tIDR plus under 1 per settle | Negligible loss | Optional sweepDust() owner-only limited to balance - (totalDeposited - totalClaimed) |
Verified: test_KnownIssue_DistributorHasNoSweep, invariant RentNeverExceedsDeposits |
| F-10 | Info | PropertyFactory, KYCRegistry, MockIDR |
The factory keeps the registrar role forever; it has no function that abuses it, but it must stay owner-gated. Ownership is single-step Ownable and one key controls everything (KYC, tIDR, rent, factory). setRegistrar, setTrustedSpender, setDistributor emit no events |
Key loss or a typo in transferOwnership is permanent; role changes are not auditable offchain |
Use Ownable2Step, a multisig, and add events to the three setters |
Verified by reading; test_FactoryLeavesNoPrivilegedKeysBehind |
| F-11 | Info | PrimarySale.buy, Marketplace.buy |
Units are transferred before payment is pulled. With a hookless ERC-20 (MockIDR) there is no reentrancy and a failed payment reverts the whole call. State (active = false, room counters) is written before external calls. If payToken were ever an ERC-777 or hook token this ordering should be revisited |
None today | Optional: pull payment first, then transfer units, and add nonReentrant |
Verified by reading |
| F-12 | Info | PropertyFactory.createProperty |
treasury may be any address (including an unrelated contract); strings are stored unbounded and unvalidated; unitPrice * units overflow only reverts the buy. Owner-only, so no user risk |
Operator misconfiguration only | Reject treasury == address(token)/sale; cap string length |
Verified by reading |
| F-13 | Info | PrimarySale |
Units sent back to the sale are resold at the fixed price and stop earning rent while there (inventory exclusion). The sale never pays holders for returned units, so returning units is a donation | None unless users misuse it | Frontend should not offer "return to sale" | Verified: test_UnitsReturnedToSaleStopEarningRent |
| U-01 | Unverified | RentDistributor.depositRent |
Very large deposits: amount * 1e18 overflows above about 1.1e59 tIDR, far beyond any realistic supply. Not tested |
None realistic | none | Unverified, reasoning only |
Checked and found sound: settle ordering (settle from and to before balances change, inventory and mint/burn skipped, so a buyer never earns rent from before they held units and a seller never loses accrued rent); holders cannot claim more than deposited; the accumulator is safe with a circulating supply of 1; claim follows checks-effects-interactions; no unverified address, nor the marketplace, distributor or factory, can ever hold units; room accounting adds up; one-shot setters (mintInventory, setDistributor, setup) cannot be repeated; settle is callable only by the token; ownership of token and distributor ends up with the operator and not the factory.
| Invariant | Test | Outcome |
|---|---|---|
Owed (all holders) plus claimed never exceeds deposited; distributor balance equals deposited minus claimed; owed never exceeds the balance; unsold inventory earns 0; dust (deposited - claimed - owed) is at most one token per settle or deposit |
invariant_RentNeverExceedsDeposits |
Pass |
Sum of balances over all wallets and contracts equals total supply, which equals totalUnits |
invariant_BalancesSumToSupply |
Pass |
| Units sit only in the sale contract or in wallets verified at receipt time (a currently unverified holder must have been revoked after receiving); marketplace, distributor, token, KYC, factory, tIDR and stranger hold none | invariant_UnitsOnlyInVerifiedHands |
Pass |
unitsLeft plus units held by buyers equals total supply |
invariant_SaleInventoryPlusHoldersEqualsSupply |
Pass |
Room raisedUnits and contributors equal the sum over wallets |
invariant_RoomAccounting |
Pass |
A stranger can never receive units (direct, via transferFrom, or via the marketplace) |
testFuzz_StrangerCanNeverReceiveUnits, invariant (c) |
Pass |
| Two-holder rent split never overpays and loses at most 2 tIDR | testFuzz_RentSplitNeverOverpays, testFuzz_ClaimEverythingLeavesOnlyDust |
Pass |
The handler randomly buys, creates rooms, transfers to wallets and contracts (including zero-value), lists, fills, cancels, deposits rent, claims, and verifies or revokes wallets. All runs use the real PropertyFactory wiring. Limits of the harness: one property, four actors, small supply (1000 units), reverts are swallowed so sequences that always revert are not flagged.
Quote these in the README:
- Everything runs on a testnet with test tokens. tIDR is minted freely by the operator and has no value.
- "KYC" is a boolean flag set by the operator. No identity data is checked or stored onchain, and the operator can verify or un-verify any wallet at any time.
- One operator key controls verification, tIDR minting, property creation and rent deposits. There is no multisig, timelock or two-step ownership transfer.
- tIDR has a trusted-spender shortcut: contracts the operator marks as trusted can spend any holder's tIDR without approval. This exists only to save a click on the test token and would not exist with a real stablecoin.
- Un-verifying a holder freezes their units and there is no recovery or forced transfer. They can still claim rent already accrued.
- Rent is distributed by balance at the moment of deposit; short-term holders around a deposit can capture rent. Rounding dust below 1 tIDR per settlement is not tracked, and small holders can be rounded to zero by zero-value transfers.
- The marketplace is non-custodial and fixed price. Listings are not re-validated until filled, so the UI may show listings that cannot be filled.
- Urunan Rooms are counters only: no escrow, target enforcement or refund.
- Unit price in the primary sale is fixed; unsold inventory cannot be withdrawn.
- The contracts have been reviewed internally and tested with invariant and fuzz tests, but have not had an external audit. The legal structure of the underlying property is out of scope.
The contract review above did not cover the Next.js API routes that hold the operator key. They were hardened afterwards. What changed, and what is still a demo-grade limit:
| Area | Fixed | Still a limit |
|---|---|---|
Gas drip (/api/onboard, /api/kyc) |
One drip per Privy user id. The claim is a non-overwritable Vercel Blob (claims/drip-<sha256>.json), so the write itself decides the winner even across serverless instances. Also: 5 requests per 10 min per IP, at most 20 drips per hour in total (MAX_DRIPS_PER_HOUR), and no drip when the wallet already holds enough gas. |
The IP and hourly counters are in memory per instance and reset on a cold start. Without a Blob token only the in-memory claim protects. A user who spends the drip and is below the gas floor gets no second drip. Anyone can still create many Privy accounts (new emails) to get one drip each, bounded by the hourly cap. |
Operator code (/api/rent, /api/listing, /api/upload) |
Compared with crypto.timingSafeEqual over SHA-256 of both sides. After 5 wrong codes an IP is locked out for 15 minutes, and each wrong attempt costs 0.6 s. |
One shared passcode for everyone who can list or deposit; no per-person identity, no rotation, no audit trail. Lockout state is in memory per instance, and the IP comes from x-forwarded-for, so a distributed attacker is only slowed down. |
Top-up (/api/topup) |
Amount must be an integer Rp10.000 to Rp1.000.000. Per-user lock so parallel requests cannot race the balance check and mint. Daily budget of Rp2.000.000 per Privy user, and 20 requests per 10 min per IP. The existing cap of Rp5.000.000 balance per wallet stays. | The daily budget is in memory per instance. New accounts get a fresh budget. tIDR has no value, so the real cost is only gas for the mint. |
Rent deposit (/api/rent) |
Amount must be a positive integer number. One deposit may pay at most 5% of the unit price per circulating unit (Rp500 per unit at the Rp10.000 demo price). The unit price is read from the property's PrimarySale, the units from RentDistributor.circulating(). Limits are exported from lib/rentLimits.ts for the UI. |
The cap is per deposit, not per month: the code holder can deposit repeatedly. It limits typos and a single-call drain; it is not a substitute for a multisig. |
Listing rehost (/api/listing) |
https only, port 443 only, host must not be localhost or resolve to a private, loopback, link-local or metadata address. Redirects are followed by hand (max 3) and each hop is checked again. Only JPEG, PNG and WebP, content-length checked, body read with a 4 MB cap. 10 requests per 10 min per IP. |
DNS rebinding between our check and the fetch is not closed (the route is behind the operator code). Pasted images are not decoded or re-encoded. |
Upload (/api/upload) |
Only JPEG, PNG and WebP, checked by declared type and by file signature; stored name and extension come from the type, not from the user. 4 MB cap, 30 uploads per 10 min per IP. | Files are public in the Blob store. No virus scan. |
RPC proxy (/api/rpc) |
Only a whitelist of read methods plus eth_sendRawTransaction; body up to 64 KB; batches up to 20; 600 requests per minute per IP; upstream timeout and errors returned as JSON-RPC errors. |
CORS stays * (the proxy is public by design and can only relay whitelisted calls). Rate limit is per instance. |
Honest summary: these measures stop casual abuse and accidents (draining the operator's gas, repeating mints, fat-fingered rent, SSRF through the image link). They do not make the operator key safe against a determined attacker: the key still sits in a serverless environment variable, one passcode guards the owner area, and in-memory counters are not shared between instances. A production version would need per-user owner roles, a multisig or HSM behind the operator, and a shared store (Redis/KV) for rate limits.