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

Sparrow_Jac - Protocol supports stETH but doesn't consider its unique transfer logic which would lead to overvaluation of deposited assets #241

Closed
sherlock-admin4 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-admin4
Copy link
Contributor

sherlock-admin4 commented Jun 27, 2024

Sparrow_Jac

High

Protocol supports stETH but doesn't consider its unique transfer logic which would lead to overvaluation of deposited assets

Summary

The StakingModule contract handles staking operations by converting weth to stETH and wstETH. However, a subtle issue arises from the "1-2 wei corner case" with stETH transfers, where the amount transferred can be slightly less than specified. This discrepancy can lead to overvaluation of deposited assets and flawed logic in subsequent operations.

Vulnerability Detail

The core issue lies in the _wethToWSteth function, which does not account for the exact amount of stETH received after converting weth. Due to a known "1-2 wei corner case" in stETH transfers, the actual amount received can be less than expected, causing miscalculations in asset values. This leads to an overvaluation of deposited stETH when passed into other functions that rely on exact amounts.
For added context/ reference, take a look at:

Impact

This discrepancy can cause several issues:

  • Overvaluation: The protocol may overvalue the deposited assets, leading to imprecise calculations in various staking and withdrawal operations.
  • Boost Logic Flaws: If the protocol allows boosting based on deposited amounts, the overvaluation could allow users to boost more than they should be able to, leading to potential financial imbalances.

Code Snippet

The current implementation of _wethToWSteth does not handle the exact amount of stETH received:
https://github.com/sherlock-audit/2024-06-mellow/blob/main/mellow-lrt/src/modules/obol/StakingModule.sol#L77-L83

Tool used

Manual Review

Recommendation

To mitigate this issue, modify _wethToWSteth to check and use the exact amount of stETH received:

function _wethToWSteth(uint256 amount) private {
    IWeth(weth).withdraw(amount);
    
    uint256 initialStethBalance = IERC20(steth).balanceOf(address(this));
    ISteth(steth).submit{value: amount}(address(0));
    uint256 receivedSteth = IERC20(steth).balanceOf(address(this)) - initialStethBalance;
    
    IERC20(steth).safeIncreaseAllowance(address(wsteth), receivedSteth);
    IWSteth(wsteth).wrap(receivedSteth);
}

Duplicate of #299

@sherlock-admin3 sherlock-admin3 changed the title Beautiful Teal Kookaburra - emergencyWithdraw does not charging withdrawal fee like process withdrawal Protocol supports stETH but doesn't consider its unique transfer logic which would lead to overvaluation of deposited assets 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 Protocol supports stETH but doesn't consider its unique transfer logic which would lead to overvaluation of deposited assets Ripe Gingerbread Dog - Protocol supports stETH but doesn't consider its unique transfer logic which would lead to overvaluation of deposited assets 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 Ripe Gingerbread Dog - Protocol supports stETH but doesn't consider its unique transfer logic which would lead to overvaluation of deposited assets Sparrow_Jac - Protocol supports stETH but doesn't consider its unique transfer logic which would lead to overvaluation of deposited assets 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

2 participants