OpenZeppelin, in its role as Security Partner to the Compound DAO, reviewed the proposal to add a wstETH market on mainnet.
Summary
Timeline: 2024-08-29 - 2024-09-03
Total Issues: 6 (5 resolved)
Medium Severity Issues: 1 (1 resolved)
Low Severity Issues: 4 (4 resolved)
Notes & Additional Information: 1 (0 resolved)
Scope
OpenZeppelin reviewed pull request #911 of the compound-finance/comet repository at commit 98fc0c2. This pull request will submit a governance proposal to create a wstETH Comet market on the mainnet using the official Comet Migration process.
In scope was the governance proposal created by the simulated Deploy Market Enact Workflow, using the migration files:
.github/workflows/enact-migration.yaml
deployments/mainnet/wsteth/migrations/1723732097_configure_and_ens.ts
and the MainnetBulkerWithWstETHSupport contract implemented for the wstETH market:
contracts/bulkers/MainnetBulkerWithWstETHSupport.sol
System Overview
As described in the Compound Community Forum Proposal, the Compound Growth Program proposes launching the wstETH market on Mainnet. The current scope for the wstETH Comet market includes rsETH and ezETH as collateral assets.
To deploy an official market on Mainnet, a series of specific Compound Governance steps must be taken. Dependencies must be accurately and securely deployed after which an official governance proposal is to be proposed on-chain. The focus of OpenZeppelin’s review was Pull Request 911 which includes the deployed dependencies, the official migration governance proposal, and all the security concerns that may arise during execution of the proposal. Specifically we checked:
- That correct contracts were deployed on-chain.
- That the simulation workflow created the same proposal as the deployment workflow.
- That the proposal description was accurate and descriptive of its effects.
- That proposal dependencies were verified as deployed with correct configurations.
- That proposal instructions were decoded and verified as complete.
- That correct oracle price feeds were used.
- That proposal instructions were simulated and executed successfully without errors.
- That the eventual market configuration was verified to match the parameter values provided by Gauntlet and accepted by the community.
Additionally, a bulker contract has been implemented to facilitate wstETH market multi-call operations. The MainnetBulkerWithWstETHSupport contract builds on top of the BaseBulker, adding supply and withdrawal functionality for the wstETH Mainnet Comet market by wrapping and unwrapping stETH tokens.
Security Model and Trust Assumptions
The MainnetBulkerWithWstETHSupport contract is assumed to be only used for the wstETH Mainnet Comet market. The bulker functionality has been specifically designed for the cwstETHv3 Comet. Consequently, utilizing it for other markets — where, for instance, wstETH might be a collateral asset — could result in incorrect functionality. In addition, the bulker contract could malfunction if used on other networks (e.g., networks where stETH is not deployed).
Before proposal enactment to incorporate the wstETH market on Mainnet, it is assumed that there will not be any upgrades or changes to the implementations of the proposal. It is also assumed that any changes from the Comet ecosystem at large such as any other governance proposals between this audit and this proposal execution should not have any effects.
Privileged Roles
The MainnetBulkerWithWstETHSupport contract has an admin role that can take out the tokens mistakenly transferred to the contract.
Medium Severity
ACTION_WITHDRAW_STETH Fails to Support Full wstETH Withdrawals
The withdrawStEthTo function allows users to withdraw wstETH from the cwstETHv3 market by specifying a stETHAmount. The corresponding wstETH amount is computed, withdrawn from the Comet, and unwrapped to stETH. If a type(uint256).max amount is specified, the function should withdraw the user’s entire balance. However, instead of retrieving the balance of the wstETH base asset for the user, the function computes the collateral amount of wstETH, which would be 0 in the wstETH market, preventing the user from withdrawing the whole position.
Consider ensuring that the Comet base asset is wstETH and querying the Comet for the balance of the base asset using CometInterface(comet).balanceOf(msg.sender) instead of querying the collateral balance with CometInterface(comet).collateralBalanceOf(msg.sender, wsteth).
Update: Resolved in commit ca245ce.
Low Severity
Missing Validation in deposit Function
The deposit function allows anyone to deposit ETH. The ETH value is deposited into the wstETH contract via the receive function, sending wstETH tokens to the contract, which are then transferred to the specified Comet address. This function is used during deployment to fund the Comet using ETH. However, the function could be misleading to users.
Consider ensuring that the deposit function is only callable by the admin and that the comet parameter address is the cwstETHv3 address.
Update: Resolved in commit ca245ce.
Misleading Documentation
The docstrings of the supplyStEthTo and withdrawStEthTo functions mention that the wstETH base asset is not supported even though the MainnetBulkerWithWstETHSupport contract has been designed for the cwstETHv3 market.
Consider updating the function docstrings to reflect the current implementation.
Update: Resolved in commit ca245ce addressing all instances in lines 67 and 85.
ACTION_SUPPLY_STETH Fails to Support Full Loan Repayments
The supplyStEthTo function allows users to supply wstETH to the cwstETHv3 market by specifying a stETHAmount. The stETH amount is wrapped to wstETH and supplied to the Comet. Since cwstETHv3 is a rebasing token and the present value of a loan is constantly increasing, it is practically impossible for users to fully repay their loans by calculating the corresponding stETHAmount required at the time of execution.
We recommend accepting a stETHAmount of type(uint256).max, querying the Comet for the balance of the outstanding loan using CometInterface(comet).borrowBalanceOf(msg.sender), converting the wstETH borrow balance to stETH using IWstETH(wstETH).getStETHByWstETH, and repaying the user loan in full.
Update: Resolved in commit ca245ce.
Unused Variable lastValue
The lastValue variable is a public uint256-type variable that is initialized with 1. Every time the deposit function is called, the lastValue is set to the wstETHAmount returned as a result of the deposit of ETH to the wstETH contract.
Consider removing the unused variable to make the implementation more clear and concise.
Update: Resolved in commit ca245ce.
Notes & Additional Information
weETH is not Listed As a Collateral Asset
Contrary to the wstETH Market discussion, weETH is not listed as a collateral asset to the wstETH Comet on Mainnet.
We recommend adding the missing collateral asset or discussing the reasons for its omission publicly to ensure transparency within the community.
Conclusion
The MainnetBulkerWithWstETHSupport contract has been implemented to support multi-call operations in the wstETH Mainnet Comet market, adding supply and withdrawal functionality by wrapping and unwrapping stETH tokens. It is specifically designed for the cwstETHv3 Comet and may not function correctly if used on networks where stETH is not deployed.
The audit yielded 1 medium- and 4 low-severity issues. Upon reviewing the calldata generated by the migration script we have identified 1 note. Otherwise, the proposal has been correctly implemented and follows all current recommendations.
. 