Permanent infinite approval of assets gives EigenLayer Strategy Manager more access than necessary

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

The NodeDelegator.maxApproveToEigenStrategyManager() function sets an infinite approval for the corresponding asset to the EigenLayer Strategy Manager.
In the current implementation this function must be called in order to deposit the asset into the EigenLayer Strategy Manager. There is no functionality to revoke or reduce the approval.
In case the EigenLayer Strategy Manager was compromised, this approval would put the full amount of the asset held by the NodeDelegator at risk, even if the NodeDelegator contract was paused. This drastically reduces the effectiveness of such countermeasures.

Proof of Concept

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

IERC20(asset).approve(eigenlayerStrategyManagerAddress, type(uint256).max);

Tools Used

Manual review.

Recommended Mitigation Steps

• Instead of setting a permanent infinite approval once, set an approval for the specific amount that should be deposited into the EigenLayer Strategy Manager in NodeDelegator.depositAssetIntoStrategy() before depositIntoStrategy() is called.

Assessed type

ERC20

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #70

[c4-judge]

fatherGoose1 changed the severity to QA (Quality Assurance)

[c4-judge]

fatherGoose1 marked the issue as grade-b