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

RadCet - stEth::transferFrom will transfer 1-2 less way result in revert in deposit function #141

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

RadCet

High

stEth::transferFrom will transfer 1-2 less way result in revert in deposit function

Summary

Many cases stEth::transferFrom will transfer 1-2 less way, which would result in revert in consequent functions, because of not enough balance

Vulnerability Detail

In DepositWrapper:deposit, user call this function to deposit weth, steth,.. However when user try to deposit stETH, contract using IERC20(steth).safeTransferFrom(sender, wrapper, amount); to send seETH from user to contract. stETH is using shares for tracking balances and it is a known issue that due to rounding error, transferred shares may be 1-2 wei less than _amount passed. So when call _DepositWrapper:stethToWsteth will be revert due to not enough balance.

Impact

Deposit function for stETH is DOS
Possibility: The probability of issue appearing is high and you can check in the following discussion. It has also been classified as a High severity on past contests: lidofinance/lido-dao#442

Code Snippet

DepositWrapper:deposit

    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);
            amount = _stethToWsteth(amount);
        } else if (token == weth) {
    function _stethToWsteth(uint256 amount) private returns (uint256) {
        IERC20(steth).safeIncreaseAllowance(wsteth, amount);
        IWSteth(wsteth).wrap(amount);
        return IERC20(wsteth).balanceOf(address(this));
    }

Tool used

Manual Review

Recommendation

Use lido recommendation to utilize transferShares function, so the _amount is realistic, or compares the balance before and after the transfer.

Duplicate of #299

@sherlock-admin4 sherlock-admin4 changed the title Rapid Violet Scorpion - RestrictingKeeper::processConfigurators() lacks call permission restrictions, anyone can perform revocation, and segmentation operations will not be performed as expected stEth::transferFrom will transfer 1-2 less way result in revert in deposit function 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 stEth::transferFrom will transfer 1-2 less way result in revert in deposit function Funny Gunmetal Marmot - stEth::transferFrom will transfer 1-2 less way result in revert in deposit function 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 Funny Gunmetal Marmot - stEth::transferFrom will transfer 1-2 less way result in revert in deposit function RadCet - stEth::transferFrom will transfer 1-2 less way result in revert in deposit function 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