sherlock-audit / 2024-05-napier-update-judging

8 stars 7 forks source link

KupiaSec - Potential Incompatibility Issue with `PufETHAdapter::_stake` Function #73

Closed sherlock-admin2 closed 3 months ago

sherlock-admin2 commented 3 months ago

KupiaSec

medium

Potential Incompatibility Issue with PufETHAdapter::_stake Function

Summary

The PufETHAdapter::_stake function is used to stake ether and this function calls puffer depositor's depositStETH function. However the implementation is not compatible with the deployed puffer depositor's depositStETH and it will always revert.

The PufETHAdapter::_stake function is responsible for staking Ether. This function calls the depositStETH function on the Puffer Depositor contract to handle the staking process.

However, the implementation of the PufETHAdapter::_stake function is incompatible with the deployed version of the Puffer Depositor's depositStETH function. Whenever the PufETHAdapter attempts to call depositStETH, the transaction will revert, causing the entire _stake operation to fail.

Vulnerability Detail

The PufETHAdapter::_stake calls the depositStETH function on the PUFFER_DEPOSITOR contract at L82.


PufETHAdapter.sol
66: function _stake(uint256 stakeAmount) internal override returns (uint256) {
67:         if (stakeAmount == 0) return 0;
68: 
69:         uint256 stakeLimit = STETH.getCurrentStakeLimit();
70:         if (stakeAmount > stakeLimit) {
71:             // Cap stake amount
72:             stakeAmount = stakeLimit;
73:         }
74: 
75:         IWETH9(Constants.WETH).withdraw(stakeAmount);
76:         uint256 _stETHAmt = STETH.balanceOf(address(this));
77:         STETH.submit{value: stakeAmount}(address(this));
78:         _stETHAmt = STETH.balanceOf(address(this)) - _stETHAmt;
79:         if (_stETHAmt == 0) revert InvariantViolation();
80: 
81:         // Stake stETH to PufferDepositor
82:         uint256 _pufETHAmt = PUFFER_DEPOSITOR.depositStETH(Permit(block.timestamp, _stETHAmt, 0, 0, 0)); // @audit-issue  incompatible with the current pufferDepositor v2
83: 
84:         if (_pufETHAmt == 0) revert InvariantViolation(); 
85: 
86:         return stakeAmount;
87:     }

The implementation of the PUFFER_DEPOSITOR.depositStETH function is as follows:


    function depositStETH(Permit calldata permitData, address recipient)
        external
        restricted
        returns (uint256 pufETHAmount)
    {
        try ERC20Permit(address(_ST_ETH)).permit({
            owner: msg.sender,
            spender: address(this),
            value: permitData.amount,
            deadline: permitData.deadline,
            v: permitData.v,
            s: permitData.s,
            r: permitData.r
        }) { } catch { }

        // Transfer stETH from user to this contract. The amount received here can be 1-2 wei lower than the actual permitData.amount
        SafeERC20.safeTransferFrom(IERC20(address(_ST_ETH)), msg.sender, address(this), permitData.amount);

        // The PufferDepositor is not supposed to hold any stETH, so we sharesOf(PufferDepositor) to the PufferVault immediately
        return PUFFER_VAULT.depositStETH(_ST_ETH.sharesOf(address(this)), recipient);
    }

You can check the code here.

As evident from the code snippet, the depositStETH function on the PUFFER_DEPOSITOR contract expects two parameters, but the current implementation of the PufETHAdapter::_stake function only passes a single parameter.

As a result, any attempt to call the PufETHAdapter::_stake function will consistently revert due to the incompatibility between the input parameters passed by the function and the expected parameters of the depositStETH function.

Impact

The PufETHAdapter::_stake function, which is responsible for staking Ether on behalf of users, is fundamentally flawed and will always revert when invoked.

Code Snippet

https://github.com/sherlock-audit/2024-05-napier-update/blob/c31af59c6399182fd04b40530d79d98632d2bfa7/napier-uups-adapters/src/adapters/puffer/PufETHAdapter.sol#L66-L87

Tool used

Manual Review

Recommendation

It is recommended to fix the _stake function as follows:


PufETHAdapter.sol
66: function _stake(uint256 stakeAmount) internal override returns (uint256) {
67:         if (stakeAmount == 0) return 0;
68: 
69:         uint256 stakeLimit = STETH.getCurrentStakeLimit();
70:         if (stakeAmount > stakeLimit) {
71:             // Cap stake amount
72:             stakeAmount = stakeLimit;
73:         }
74: 
75:         IWETH9(Constants.WETH).withdraw(stakeAmount);
76:         uint256 _stETHAmt = STETH.balanceOf(address(this));
77:         STETH.submit{value: stakeAmount}(address(this));
78:         _stETHAmt = STETH.balanceOf(address(this)) - _stETHAmt;
79:         if (_stETHAmt == 0) revert InvariantViolation();
80: 
81:         // Stake stETH to PufferDepositor
- 82:         uint256 _pufETHAmt = PUFFER_DEPOSITOR.depositStETH(Permit(block.timestamp, _stETHAmt, 0, 0, 0));
+ 82        uint256 _pufETHAmt = PUFFER_DEPOSITOR.depositStETH(Permit(block.timestamp, _stETHAmt, 0, 0, 0), address(this));
83: 
84:         if (_pufETHAmt == 0) revert InvariantViolation(); 
85: 
86:         return stakeAmount;
87:     }

Duplicate of #21