cyclofinance / cyclofinance/cyclo.sol
Add immutable CycloMintWrapper to make the safe deposit path verifiable (mitigates _msgSender spoofing, rain.vats#309)
- Dominant language
- Solidity
- Stars
- 0
- Forks
- 1
- PR merge metrics
- No merged PRs in 30d
Description
## Context
The receipt contract Cyclo inherits (`ERC20PriceOracleReceipt` → rain.vats `Receipt`) is vulnerable to `_msgSender()` spoofing via ERC1155 acceptance callbacks — see the Protofire r2.0 audit finding **H01** (tracked as rainlanguage/rain.vats#309).
Summary of the bug: `managerMint(sender, account, …)` runs under `withSender(sender)`, so for the whole call `Receipt._msgSender()` returns the spoofed depositor. `_mint` fires `onERC1155Received` on the recipient **before** the call returns. If the recipient is attacker-controlled, it can re-enter and call `setApprovalForAll(attacker, true)` (or `safeTransferFrom(victim, attacker, …)`) and have it attributed to the **depositor**. A victim socially-engineered into depositing with a malicious contract as `receiver` loses operator approval over — and can have drained — *all* their receipts (present, via an atomic same-tx transfer, and future, via the lingering approval until revoked).
**Cyclo cannot mitigate this at the protocol/admin level:**
- `CycloVault` is an `ERC20PriceOracleReceiptVault`, which inherits the **no-op** `ReceiptVault.authorizeReceiptTransfer3` — receipts are unconditionally transferable, there is no per-transfer authorization hook and no `setAuthorizer`.
- Cyclo is **immutable with no admin keys** — even the offchain vault's swappable-authorizer seam wouldn't be reachable.
- A fresh deployment does not help the existing TVL.
The spoof window only ever opens when a user **themselves** routes a receipt mint into attacker-controlled code as `receiver`. So the practical, deployable mitigation is to make the safe deposit path a single verifiable contract.
## Proposal: a minimal, immutable `CycloMintWrapper`
A pass-through deposit wrapper that **exposes no `receiver` parameter**:
```
deposit(assets, …minShareRatio/oracle params…):
collateral.transferFrom(msg.sender, address(this), assets)
collateral.approve(vault, assets)
shares = vault.deposit(assets, address(this), …) // receiver is ALWAYS address(this)
// wrapper now holds the cyclo shares (ERC20) + the receipt (ERC1155)
cycloShare.transfer(msg.sender, shares)
receipt.safeTransferFrom(address(this), msg.sender, id, shares, "")
```
Why it's safe:
- The only acceptance callback fired while a sender is spoofed goes to the **wrapper itself** (it is the `receiver`); the wrapper's `onERC1155Received` is a benign `return selector`. No attacker code runs inside the spoof window.
- The hand-back `safeTransferFrom(wrapper → user)` is **not** under `withSender`, so `_msgSender()` is the truthful wrapper; no spoofing.
- The wrapper holds **no standing balance** (everything is pulled and forwarded within one call) — nothing to steal from the wrapper, trivial to audit.
- Because the wrapper never exposes a `receiver` knob, a user calling it **cannot** route receipts to a third party. The attack then requires the user to bypass the wrapper entirely and call the raw vault with a malicious `receiver` — a far more conspicuous ask than "approve this staking contract."
This collapses the user's trust decision from "audit every possible recipient" to "am I calling the canonical wrapper address?".
### Scope / honest limits (document, don't hide)
- It protects the **deposit action**, not already-held EOA balances: receipts are handed back to the user's EOA, so a user later phished into bypassing the wrapper is still exposed. (A custody-style wrapper would isolate standing balance but adds withdrawal-routing complexity — out of scope here.)
- It is opt-in and cannot be imposed; the vaults remain permissionless.
- It does not recover assets in a deposit the user is actively tricked into making.
### Acceptance criteria
- [ ] `CycloMintWrapper` deployed per oracle/asset vault, immutable, no admin, holds no standing balance.
- [ ] No `receiver`/`to` parameter on the deposit entrypoint; always mints to `address(this)` then forwards to `msg.sender`.
- [ ] Benign `onERC1155Received` (returns the magic value, no external calls / no state).
- [ ] Tests: a wrapper deposit grants no spoofable approval and drains no pre-existing receipts; re-running the #309 PoC through the wrapper has no foothold.
- [ ] Pairs with a docs task on cyclo.site (filed separately) telling users to always deposit via the wrapper and how to verify its address.
Underlying finding + escalation/mitigation tests: rainlanguage/rain.vats#309.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.