Skip to content

Latest commit

 

History

History
142 lines (120 loc) · 6.59 KB

File metadata and controls

142 lines (120 loc) · 6.59 KB

WstEth and SfrxEth derivatives don't verify staking requirements

Impact

The WstEth derivative is used to stake ETH in the Lido protocol. The deposit function takes an ETH transfer from the SafEth contract and forwards it to the Lido wstETH contract:

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/WstEth.sol#L73-L81

function deposit() external payable onlyOwner returns (uint256) {
    uint256 wstEthBalancePre = IWStETH(WST_ETH).balanceOf(address(this));
    // solhint-disable-next-line
    (bool sent, ) = WST_ETH.call{value: msg.value}("");
    require(sent, "Failed to send Ether");
    uint256 wstEthBalancePost = IWStETH(WST_ETH).balanceOf(address(this));
    uint256 wstEthAmount = wstEthBalancePost - wstEthBalancePre;
    return (wstEthAmount);
}

In wstETH, the staking and minting of stETH will be handled by the submit function in the Lido contract (which inherits from stETH):

https://github.com/lidofinance/lido-dao/blob/master/contracts/0.6.12/WstETH.sol#L80-L83

receive() external payable {
    uint256 shares = stETH.submit{value: msg.value}(address(0));
    _mint(msg.sender, shares);
}

Expanding on the _submit function:

https://github.com/lidofinance/lido-dao/blob/master/contracts/0.4.24/Lido.sol#L675-L705

675:     function _submit(address _referral) internal returns (uint256) {
676:         require(msg.value != 0, "ZERO_DEPOSIT");
677: 
678:         StakeLimitState.Data memory stakeLimitData = STAKING_STATE_POSITION.getStorageStakeLimitStruct();
679:         require(!stakeLimitData.isStakingPaused(), "STAKING_PAUSED");
680: 
681:         if (stakeLimitData.isStakingLimitSet()) {
682:             uint256 currentStakeLimit = stakeLimitData.calculateCurrentStakeLimit();
683: 
684:             require(msg.value <= currentStakeLimit, "STAKE_LIMIT");
685: 
686:             STAKING_STATE_POSITION.setStorageStakeLimitStruct(
687:                 stakeLimitData.updatePrevStakeLimit(currentStakeLimit - msg.value)
688:             );
689:         }
690: 
691:         uint256 sharesAmount = getSharesByPooledEth(msg.value);
692:         if (sharesAmount == 0) {
693:             // totalControlledEther is 0: either the first-ever deposit or complete slashing
694:             // assume that shares correspond to Ether 1-to-1
695:             sharesAmount = msg.value;
696:         }
697: 
698:         _mintShares(msg.sender, sharesAmount);
699: 
700:         BUFFERED_ETHER_POSITION.setStorageUint256(_getBufferedEther().add(msg.value));
701:         emit Submitted(msg.sender, msg.value, _referral);
702: 
703:         _emitTransferAfterMintingShares(msg.sender, sharesAmount);
704:         return sharesAmount;
705:     }

As we can in the previous snippet, there are several conditions and requirements that will make the deposit fail:

  1. Lines 678-679 check the settings for the isStakingPaused flag. If staking is currently paused, the operation will fail.
  2. Lines 681-689 check that the requested amount is within the stake limits. If there's a limit set and the requested amount is beyond the limit, the operation will fail.
  3. An edge case but technically valid is the check in line 676 for msg.value != 0. Staking in SafEth divides the deposit amount by weight. If the amount and weighted proportion for WstEth are low, the division may be rounded to 0.

Similarly, in the SfrxEth derivative, the deposit action is handled by the submitAndDeposit of the FRAX contract frxETHMinter:

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/SfrxEth.sol#L94-L106

function deposit() external payable onlyOwner returns (uint256) {
    IFrxETHMinter frxETHMinterContract = IFrxETHMinter(
        FRX_ETH_MINTER_ADDRESS
    );
    uint256 sfrxBalancePre = IERC20(SFRX_ETH_ADDRESS).balanceOf(
        address(this)
    );
    frxETHMinterContract.submitAndDeposit{value: msg.value}(address(this));
    uint256 sfrxBalancePost = IERC20(SFRX_ETH_ADDRESS).balanceOf(
        address(this)
    );
    return sfrxBalancePost - sfrxBalancePre;
}

As we can see in the following snippet, the staking in the FRAX protocol has also some requirements:

https://github.com/FraxFinance/frxETH-public/blob/master/src/frxETHMinter.sol#L70-L101

070:     function submitAndDeposit(address recipient) external payable returns (uint256 shares) {
071:         // Give the frxETH to this contract after it is generated
072:         _submit(address(this));
073: 
074:         // Approve frxETH to sfrxETH for staking
075:         frxETHToken.approve(address(sfrxETHToken), msg.value);
076: 
077:         // Deposit the frxETH and give the generated sfrxETH to the final recipient
078:         uint256 sfrxeth_recieved = sfrxETHToken.deposit(msg.value, recipient);
079:         require(sfrxeth_recieved > 0, 'No sfrxETH was returned');
080: 
081:         return sfrxeth_recieved;
082:     }
083: 
084:     /// @notice Mint frxETH to the recipient using sender's funds. Internal portion
085:     function _submit(address recipient) internal nonReentrant {
086:         // Initial pause and value checks
087:         require(!submitPaused, "Submit is paused");
088:         require(msg.value != 0, "Cannot submit 0");
089: 
090:         // Give the sender frxETH
091:         frxETHToken.minter_mint(recipient, msg.value);
092: 
093:         // Track the amount of ETH that we are keeping
094:         uint256 withheld_amt = 0;
095:         if (withholdRatio != 0) {
096:             withheld_amt = (msg.value * withholdRatio) / RATIO_PRECISION;
097:             currentWithheldETH += withheld_amt;
098:         }
099: 
100:         emit ETHSubmitted(msg.sender, recipient, msg.value, withheld_amt);
101:     }
  1. In line 87 there's a check for a pause setting. If staking is currently paused, the operation will fail.
  2. Similar to the edge case described above, line 88 verifies that msg.value != 0. The division in the weighted proportion may round down the amount to 0.

In summary, this means that if any of these conditions during the staking process of the WstEth or SfrxEth derivative isn't met, then the deposit will fail and will block the whole staking and rebalancing processes in the SafEth contract.

Recommendation

Similar to how it is implemented in the Reth derivative, the deposit function of the WstEth and SfrxEth derivative can check the current state of the different staking requirements and decide between staking in the respective protocol (Lido or Frax) or taking an alternative path, such as swapping ETH for stETH using the Curve pool in the case of WstEth or swapping ETH for frxETH in the case of SfrxEth.