Saving balance of token in variable can lead to wrong amount transfer or failure

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

Saving contract balance into variable will result in wrong price being transferred using the depositIntoStrategy method which will cause the transfer to fail if balance is less than transferred and wrong amount transferred if amount increases.

Proof of Concept

The balance of token is saved into balance variable which is then used to transfer using the depositIntoStrategy method.

    function depositAssetIntoStrategy(address asset)
        external
        override
        whenNotPaused
        nonReentrant
        onlySupportedAsset(asset)
        onlyLRTManager
    {
        address strategy = lrtConfig.assetStrategy(asset);
        IERC20 token = IERC20(asset);
        address eigenlayerStrategyManagerAddress = lrtConfig.getContract(LRTConstants.EIGEN_STRATEGY_MANAGER);

        uint256 balance = token.balanceOf(address(this));

        emit AssetDepositIntoStrategy(asset, strategy, balance);

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

Tools Used

Manual Review

Recommended Mitigation Steps

Use token balance in the contract address directly to avoid wrong transfer.

    function depositAssetIntoStrategy(address asset)
        external
        override
        whenNotPaused
        nonReentrant
        onlySupportedAsset(asset)
        onlyLRTManager
    {
        address strategy = lrtConfig.assetStrategy(asset);
        IERC20 token = IERC20(asset);
        address eigenlayerStrategyManagerAddress = lrtConfig.getContract(LRTConstants.EIGEN_STRATEGY_MANAGER);

-        uint256 balance = token.balanceOf(address(this));

-        emit AssetDepositIntoStrategy(asset, strategy, balance);
+   emit AssetDepositIntoStrategy(asset, strategy, token.balanceOf(address(this));

- IEigenStrategyManager(eigenlayerStrategyManagerAddress).depositIntoStrategy(IStrategy(strategy), token, balance);
+ IEigenStrategyManager(eigenlayerStrategyManagerAddress).depositIntoStrategy(IStrategy(strategy), token, token.balanceOf(address(this));
    }

Assessed type

Other

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as primary issue

[raymondfam]

Nothing wrong here. Caching is meant for gas saving.

[c4-judge]

fatherGoose1 marked the issue as unsatisfactory: Invalid