Attacker can mint rsETH without sending tokens

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L119-L144 https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L109 https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTOracle.sol#L78

Vulnerability details

Bug Description

The stETH token has a 1 wei corner case problem. This basically is that the transferred amount will be 1-2 we less than expected when introducing an amount to transfer. This is because an integer division happens and rounding down applies. You can read more about it in the official documentation: https://docs.lido.fi/guides/lido-tokens-integration-guide/#1-2-wei-corner-case

This is the code of the StETH.sol contract: https://github.com/lidofinance/lido-dao/blob/master/contracts/0.4.24/StETH.sol

This is the transfer() function of stETH:

    function transfer(address _recipient, uint256 _amount) external returns (bool) {
        _transfer(msg.sender, _recipient, _amount);
        return true;
    }

The amount to be transffered, _sharesToTransfer is calculated by getSharesByPooledEth() as we can see in the internal _transfer() method:

    function _transfer(address _sender, address _recipient, uint256 _amount) internal {
        uint256 _sharesToTransfer = getSharesByPooledEth(_amount);
        _transferShares(_sender, _recipient, _sharesToTransfer);
        _emitTransferEvents(_sender, _recipient, _amount, _sharesToTransfer);
    }

The function getSharesByPooledEth() is as follows:

    function getSharesByPooledEth(uint256 _ethAmount) public view returns (uint256) {
        return _ethAmount
            .mul(_getTotalShares())
            .div(_getTotalPooledEther());
    }

If we check Etherscan, we can see that this method will return a 0 when trying to transfer 1 wei. This means, that if a user tries to deposit 1 wei of stETH, in reality 0 tokens will be transferred. You can check that this statement hold by reading from the StETH.sol deployed contract: https://etherscan.io/token/0xae7ab96520de3a18e5e111b5eaab095312d7fe84#readProxyContract

This will lead to a several issue as I will explain below.

Impact

An attacker can mint rsETH without sending tokens, which will result in all users receiving less rsETH than expected when depositing funds.

Proof of Concept

Taking into account the edge case where a user transferring 1 wei of stETH will result in no funds being transferred, an attacker can mint rsETH without sending tokens as follows:

1. Attacker calls depositAsset() function with asset == stETH and depositAmount == 1 wei.
2. Attacker gets some rsETH tokens, but there where 0 funds transferred to the contract because of the rounding issue when calculating the amount of stETH tokens to be sent.
3. If this is done multiple times, the attacker will gain free rsETH tokens. Also, it will cause inconsistencies in the protocol, since all the calculations for the minting amount of rsETH will be wrong, since there will be rsETH added to the supply without depositing funds into the protocol. This will result in all users receiving less rsETH than expected when depositing funds.

Tools Used

Manual review

Recommended Mitigation Steps

As suggested in the official documentation of Lido that I attached before: To avoid it, one can use transferShares to be precise.

Assessed type

Decimal

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as primary issue

[c4-sponsor]

manoj9april (sponsor) confirmed

[manoj9april]

We are aware of this issue with stETH. Don't see any incentive behind this action at an expense of gas cost. We will add a minimum deposit amount to mitigate the above edge case.

[fatherGoose1]

This is a really good find, but I must request a POC showing either:

• Profitable execution against incurred gas fees
• Negative effects for Kelp users

[c4-judge]

fatherGoose1 marked the issue as unsatisfactory: Insufficient proof

[xAriextz13]

@fatherGoose1 I agree that this issue doesn't arise any loss of funds with the current implementation. Nonetheless, I think this issue could me graded as low/informational.

[fatherGoose1]

Hello @xAriextz13, given that the sponsor already understands the issue and you already have a Grade B QA report, this will remain as is.