Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hunter - deposit() in DepositWrapper causes multiple reverts to due unhandled conversion of stETH to wstETH #272

Closed
sherlock-admin3 opened this issue Jun 27, 2024 · 0 comments
Labels
Non-Reward This issue will not receive a payout Sponsor Disputed The sponsor disputed this issue's validity

Comments

@sherlock-admin3
Copy link
Contributor

sherlock-admin3 commented Jun 27, 2024

Hunter

High

deposit() in DepositWrapper causes multiple reverts to due unhandled conversion of stETH to wstETH

Summary

the function deposit() in DepositWrapper is supposed to make users life easier to convert their tokens to wstETH but in fact it causes multiple reverts to due unhandled conversion of stETH to wstETH

Vulnerability Detail

in DepositWrapper contract there is a function called deposit that lets the users deposit to the vault of wstETH with tokens like (ETH, wETH,wstETH)

but the problem lies in the fact that the function deposit handles the conversion between stETH to wstETH in a wrong way and leads to multiple reverts, lets see how

File: DepositWrapper.sol
42:     function deposit(
43:         address to,
44:         address token,
45:         uint256 amount,
46:         uint256 minLpAmount,
47:         uint256 deadline
48:     ) external payable returns (uint256 lpAmount) {
49:         address wrapper = address(this);
50:         address sender = msg.sender;
51:         address[] memory tokens = vault.underlyingTokens();
52:         if (tokens.length != 1 || tokens[0] != wsteth)
53:             revert InvalidTokenList();
54:         if (amount == 0) revert InvalidAmount();
55:         if (token == steth) {
56:             IERC20(steth).safeTransferFrom(sender, wrapper, amount);
57:             amount = _stethToWsteth(amount);
58:         } else if (token == weth) {
59:             IERC20(weth).safeTransferFrom(sender, wrapper, amount);
60:             amount = _wethToWsteth(amount);
61:         } else if (token == address(0)) {
62:             if (msg.value != amount) revert InvalidAmount();
63:             amount = _ethToWsteth(amount);
64:         } else if (wsteth == token) {
65:             IERC20(wsteth).safeTransferFrom(sender, wrapper, amount);
66:         } else revert InvalidToken();
67: 
68:         IERC20(wsteth).safeIncreaseAllowance(address(vault), amount);
69:         uint256[] memory amounts = new uint256[](1);
70:         amounts[0] = amount;
71:         (, lpAmount) = vault.deposit(to, amounts, minLpAmount, deadline);
72:         uint256 balance = IERC20(wsteth).balanceOf(wrapper);
73:         if (balance > 0) IERC20(wsteth).safeTransfer(sender, balance);
74:         emit DepositWrapperDeposit(sender, token, amount, lpAmount, deadline);
75:     }

in Line #55 we check if token of the deposit is stETH then we transfer the amount specified to the contract then we call _stethToWsteth() function the problem here is that

  • often times whole stETH balance can't be transferred from the account while leaving the last 1-2 wei on the sender's account as acknowledged by lido Docs here and was dicussed on their github here, no we have 1 to 2 wei less amounts transferred
    • we call _stethToWsteth() with amount input of the user

at _stethToWsteth()

File: DepositWrapper.sol
35:     function _stethToWsteth(uint256 amount) private returns (uint256) {
36:         IERC20(steth).safeIncreaseAllowance(wsteth, amount);
37:         IWSteth(wsteth).wrap(amount);
38:         return IERC20(wsteth).balanceOf(address(this));
39:     }

we just call wrap at Line #37 but the problem is that we are calling it with amount provided in deposit that our contract now holds 1 to 2 wei less than it

Impact

High, this contract is supposed to make user interactions easier but in fact it produces harm, funds loss(on gas) and large user inconvenience

Code Snippet

deposit() https://github.com/sherlock-audit/2024-06-mellow/blob/26aa0445ec405a4ad637bddeeedec4efe1eba8d2/mellow-lrt/src/utils/DepositWrapper.sol#L42-L75
File: DepositWrapper.sol
42:     function deposit(
43:         address to,
44:         address token,
45:         uint256 amount,
46:         uint256 minLpAmount,
47:         uint256 deadline
48:     ) external payable returns (uint256 lpAmount) {
49:         address wrapper = address(this);
50:         address sender = msg.sender;
51:         address[] memory tokens = vault.underlyingTokens();
52:         if (tokens.length != 1 || tokens[0] != wsteth)
53:             revert InvalidTokenList();
54:         if (amount == 0) revert InvalidAmount();
55:         if (token == steth) {
56:             IERC20(steth).safeTransferFrom(sender, wrapper, amount);
57:             amount = _stethToWsteth(amount);
58:         } else if (token == weth) {
59:             IERC20(weth).safeTransferFrom(sender, wrapper, amount);
60:             amount = _wethToWsteth(amount);
61:         } else if (token == address(0)) {
62:             if (msg.value != amount) revert InvalidAmount();
63:             amount = _ethToWsteth(amount);
64:         } else if (wsteth == token) {
65:             IERC20(wsteth).safeTransferFrom(sender, wrapper, amount);
66:         } else revert InvalidToken();
67: 
68:         IERC20(wsteth).safeIncreaseAllowance(address(vault), amount);
69:         uint256[] memory amounts = new uint256[](1);
70:         amounts[0] = amount;
71:         (, lpAmount) = vault.deposit(to, amounts, minLpAmount, deadline);
72:         uint256 balance = IERC20(wsteth).balanceOf(wrapper);
73:         if (balance > 0) IERC20(wsteth).safeTransfer(sender, balance);
74:         emit DepositWrapperDeposit(sender, token, amount, lpAmount, deadline);
75:     }

_stethToWsteth() https://github.com/sherlock-audit/2024-06-mellow/blob/26aa0445ec405a4ad637bddeeedec4efe1eba8d2/mellow-lrt/src/utils/DepositWrapper.sol#L35-L39
File: DepositWrapper.sol
35:     function _stethToWsteth(uint256 amount) private returns (uint256) {
36:         IERC20(steth).safeIncreaseAllowance(wsteth, amount);
37:         IWSteth(wsteth).wrap(amount);
38:         return IERC20(wsteth).balanceOf(address(this));
39:     }

Tool used

Manual Review

Recommendation

    function deposit(
        address to,
        address token,
        uint256 amount,
        uint256 minLpAmount,
        uint256 deadline
    ) external payable returns (uint256 lpAmount) {
        address wrapper = address(this);
        address sender = msg.sender;
        address[] memory tokens = vault.underlyingTokens();
        if (tokens.length != 1 || tokens[0] != wsteth)
            revert InvalidTokenList();
        if (amount == 0) revert InvalidAmount();
        if (token == steth) {
            IERC20(steth).safeTransferFrom(sender, wrapper, amount);
+           unint256 actualBalance = IERC20(steth).balanceOf(address(this))
-           amount = _stethToWsteth(amount);
+           amount = _stethToWsteth(actualBalance);
        } else if (token == weth) {
            IERC20(weth).safeTransferFrom(sender, wrapper, amount);
            amount = _wethToWsteth(amount);
        } else if (token == address(0)) {
            if (msg.value != amount) revert InvalidAmount();
            amount = _ethToWsteth(amount);
        } else if (wsteth == token) {
            IERC20(wsteth).safeTransferFrom(sender, wrapper, amount);
        } else revert InvalidToken();

        IERC20(wsteth).safeIncreaseAllowance(address(vault), amount);
        uint256[] memory amounts = new uint256[](1);
        amounts[0] = amount;
        (, lpAmount) = vault.deposit(to, amounts, minLpAmount, deadline);
        uint256 balance = IERC20(wsteth).balanceOf(wrapper);
        if (balance > 0) IERC20(wsteth).safeTransfer(sender, balance);
        emit DepositWrapperDeposit(sender, token, amount, lpAmount, deadline);
    }

Duplicate of #299

@sherlock-admin4 sherlock-admin4 changed the title Perfect Coral Anteater - The DepositWrapper multiple reverts in this contract due to 1 to 2 wei less transferred amount deposit() in DepositWrapper causes multiple reverts to due unhandled conversion of stETH to wstETH Jun 28, 2024
@sherlock-admin3 sherlock-admin3 added the Sponsor Disputed The sponsor disputed this issue's validity label Jun 30, 2024
@github-actions github-actions bot changed the title deposit() in DepositWrapper causes multiple reverts to due unhandled conversion of stETH to wstETH Prehistoric Snowy Corgi - deposit() in DepositWrapper causes multiple reverts to due unhandled conversion of stETH to wstETH Jul 6, 2024
@github-actions github-actions bot closed this as completed Jul 6, 2024
@github-actions github-actions bot added the Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label label Jul 6, 2024
@sherlock-admin3 sherlock-admin3 changed the title Prehistoric Snowy Corgi - deposit() in DepositWrapper causes multiple reverts to due unhandled conversion of stETH to wstETH Hunter - deposit() in DepositWrapper causes multiple reverts to due unhandled conversion of stETH to wstETH Jul 15, 2024
@sherlock-admin3 sherlock-admin3 added Non-Reward This issue will not receive a payout and removed Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Non-Reward This issue will not receive a payout Sponsor Disputed The sponsor disputed this issue's validity
Projects
None yet
Development

No branches or pull requests

1 participant