Protocol: Protocol X — illustrative sample
Scope: 9 client .sol contracts in scope (ERC-4626 vault + lending market)
Confirmed findings: 3 High 1Medium 1Low 1
Candidate observations: 1 — heuristic flags detailed in the appendix for your team to review (not yet manually confirmed).
| Severity | Definition |
|---|---|
| Critical | Funds directly at risk or guaranteed loss; exploitable with severe impact. |
| High | Loss of funds or invariant breakage under realistic conditions. |
| Medium | Bounded impact or requiring specific conditions; moderate risk. |
| Low | Minor impact, hard to exploit or with existing mitigations. |
| Info | Best practices, code quality, gas, readability. No direct risk. |
Description: `setPriceOracle(address)` is declared `external` with no access-control modifier and no inline caller check. Any account can repoint the vault's price feed to an attacker-controlled contract.
Impact: An attacker sets a malicious oracle, inflates the reported collateral price, borrows against near-worthless collateral, and drains the lending pool. Direct, unbounded loss of funds.
Recommendation: Gate `setPriceOracle` behind `onlyRole(ORACLE_ADMIN)` (ideally a multisig + timelock) and emit an `OracleUpdated` event. Consider a two-step change with a delay so integrators can react.
contracts/Vault.sol:214
213: /// @notice Update the price oracle
214: function setPriceOracle(address newOracle) external {
215: oracle = IPriceOracle(newOracle); // <-- no access control
216: }Description: `pricePerShare()` derives its value from `totalAssets()`, which reads the raw token balance held by the vault. During `withdraw()`, the ETH/token transfer to the caller happens BEFORE `totalSupply` and internal balances are finalized, so a reentrant call observes an inflated per-share value. This is the Curve/Balancer read-only-reentrancy class.
Impact: A third-party protocol that consumes `pricePerShare()` as an oracle (e.g. for collateral valuation) can be fed a manipulated price during a reentrant call, enabling under-collateralized borrowing on the integrating protocol. In-scope funds are not directly lost, but integrators relying on this view are.
Attack path: 1) Attacker deposits. 2) Calls withdraw(); the transfer triggers the attacker's fallback before balances update. 3) In the fallback, the attacker calls an integrating protocol that reads pricePerShare() -> inflated -> borrows against it.
Recommendation: Add a reentrancy read-guard to the view (revert if the nonReentrant lock is held, as Curve now does), or move all external transfers to the end of withdraw() so state is finalized first (checks-effects-interactions).
contracts/Vault.sol:181
180: function pricePerShare() public view returns (uint256) {
181: return totalAssets() * 1e18 / totalSupply(); // totalAssets() reads raw balance
182: }Description: `previewRedeem()` computes assets with a round-UP helper (`mulDivUp`). EIP-4626 requires that redeem/withdraw round DOWN in the vault's favor. Rounding up lets a redeemer extract up to 1 wei more than entitled per call.
Impact: Economically negligible per call, but repeatable; over many calls it erodes the share price against the remaining holders. This is a correctness / standards-compliance issue rather than a fund-draining bug.
Recommendation: Use a round-DOWN multiply-divide (`mulDiv`) in `previewRedeem`/`convertToAssets`. Reserve round-up for `previewMint`/`previewWithdraw`, per the EIP-4626 rounding table.
contracts/Vault.sol:167 167: return shares.mulDivUp(totalAssets(), totalSupply()); // should round DOWN on redeem
Description: `OracleAdapter.getPrice()` reads `latestRoundData()` and uses `answer` directly, without validating `updatedAt` (heartbeat) or `answer > 0`. Flagged automatically; requires confirming whether a staleness guard exists in the calling path before this is treated as confirmed.
Impact: If unguarded, a stale or zero price could be consumed on L2 sequencer downtime, enabling mispriced borrows/liquidations.
Recommendation: Verify a heartbeat check; if absent, require `block.timestamp - updatedAt <= maxDelay` and `answer > 0`.
contracts/OracleAdapter.sol:44 44: (, int256 answer,,,) = feed.latestRoundData(); // no updatedAt / answer>0 check seen
The in-scope code was reviewed against the following vulnerability classes: