code-423n4 / 2023-11-kelp-findings

13 stars 11 forks source link

IF ERC777 TOKEN (WHICH IS BACKWARD COMPATIBLE WITH ERC20) IS ADDED AS A SUPPORTED TOKEN IT COULD LEAD TO REENTRNACY ATTACK ONCE DEPOSITED INTO THE `EigenLayer` VIA THE `IEigenStrategyManager.depositIntoStrategy` FUNCTION #743

Closed c4-submissions closed 10 months ago

c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/main/src/NodeDelegator.sol#L67

Vulnerability details

Impact

The NodeDelegator.depositAssetIntoStrategy function is used to deposit an asset lying in the node delegator contract into its strategy. For the actual deposit into the strategy the IEigenStrategyManager.depositIntoStrategy function is called as shown below:

    IEigenStrategyManager(eigenlayerStrategyManagerAddress).depositIntoStrategy(IStrategy(strategy), token, balance);

The IEigenStrategyManager interface has the following Natspec comments stating that depositing the tokens that allow reentrancy (Eg: ERC777) into a strategy is not recommended, since it could lead to different attack vectors upon reentrancy.

 * WARNING: Depositing tokens that allow reentrancy (eg. ERC-777) into a strategy is not recommended.  This can lead
 * to attack vectors
 *          where the token balance and corresponding strategy shares are not in sync upon reentrancy.
 */

The documentation of the kelp protocol has mentioned that it is using ERC20 tokens. Since the ERC777 is backward compatible with ERC20 and ERC777 allows the reentrancy this could be problematic while depositing to the IEigenStrategyManager.depositIntoStrategy. If a ERC777 token is used as the asset token, this could lead to different attack vectors where the token balance and the corresponding strategy shares are manipulated via reentrancy.

Proof of Concept

        IEigenStrategyManager(eigenlayerStrategyManagerAddress).depositIntoStrategy(IStrategy(strategy), token, balance);

https://github.com/code-423n4/2023-11-kelp/blob/main/src/NodeDelegator.sol#L67

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Hence it is recommended to ensure that ERC777 asset tokens are prohibited in the kelp protocol and should not be added as a supported token in the protocol.

Assessed type

Reentrancy

c4-pre-sort commented 11 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 11 months ago

raymondfam marked the issue as primary issue

raymondfam commented 11 months ago

This would not apply to LST tokens.

c4-judge commented 10 months ago

fatherGoose1 marked the issue as unsatisfactory: Invalid