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

8 stars 7 forks source link

merlin - UniETHAdapter and RsETHAdapter do not have slippage protection #56

Closed sherlock-admin2 closed 3 months ago

sherlock-admin2 commented 3 months ago

merlin

medium

UniETHAdapter and RsETHAdapter do not have slippage protection

Summary

UniETHAdapter and RsETHAdapter have functions from external protocols that include slippage arguments. Instead of passing the minAmount argument, hardcoded zero values are used, which is incorrect.

Vulnerability Detail

RsETHAdapter has a _stake function that interacts with the RSETH_DEPOSIT_POOL.depositETH function, but the minRSETHAmountExpected is set to zero.

RSETH_DEPOSIT_POOL.depositETH{value: stakeAmount}(0, REFERRAL_ID);
function depositETH(uint256 minRSETHAmountExpected, string calldata referralId) external payable;

RSETH_DEPOSIT_POOL.depositETH minRSETHAmountExpected check;

if (rsethAmountToMint < minRSETHAmountExpected) {
            revert MinimumAmountToReceiveNotMet();
        }

Additionally, UniETHAdapter has a similar issue:

BEDROCK_STAKING.mint{value: stakeAmount}({minToMint: 0, deadline: block.timestamp + 1});
function mint(uint256 minToMint, uint256 deadline) external payable returns (uint256 minted);
--------------------------------------------------------------------------------------------------------

BEDROCK_STAKING.redeemFromValidators({
            ethersToRedeem: withdrawAmount,
-->         maxToBurn: type(uint256).max,
            deadline: block.timestamp + 1
        });
function redeemFromValidators(
        uint256 ethersToRedeem,
-->     uint256 maxToBurn,
        uint256 deadline
    ) external returns (uint256 burned);

Impact

Users do not have the ability to use slippage protection

Code Snippet

src/adapters/bedrock/UniETHAdapter.sol#L78 src/adapters/kelp/RsETHAdapter.sol#L84

Tool used

Manual Review

Recommendation

The best way to fix this issue is to provide a helper router smart contract for adapters with slippage protection.

Duplicate of #26

sherlock-admin4 commented 3 months ago

2 comment(s) were left on this issue during the judging contest.

z3s commented:

88

PNS commented:

slippage control is (will be) at the Tranche.issue level and not at the adapter level (following issue #84 in the previous contest)

WangSecurity commented 3 months ago

Since it's the duplicate of #88 and 88 will be duplicated with #26, this will also ba dup of 26.