NodeDelegator: Some tokens must approve 0 first

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

Some tokens do not work when changing the allowance from an existing non-zero allowance value (after initial max approval). They must first be approved by zero, and then the actual allowance must be approved (in this case, max approve).

Proof of Concept

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/NodeDelegator.sol#L38-L46 https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/NodeDelegator.sol#L45

Quick Reference: https://github.com/MetaMask/metamask-extension/issues/18610

Tools Used

Manual

Recommended Mitigation Steps

The function should be updated to handle the case of such tokens (tokens which require the approval amount to start from zero).

      function maxApproveToEigenStrategyManager(address asset)
        external
        override
        onlySupportedAsset(asset)
        onlyLRTManager
    {
        address eigenlayerStrategyManagerAddress = lrtConfig.getContract(LRTConstants.EIGEN_STRATEGY_MANAGER);
        +  IERC20(asset).approve(eigenlayerStrategyManagerAddress, 0);
        IERC20(asset).approve(eigenlayerStrategyManagerAddress, type(uint256).max);
    } 

Assessed type

ERC20

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as primary issue

[raymondfam]

L-06 from the bot.

[c4-judge]

fatherGoose1 marked the issue as unsatisfactory: Invalid