Custom strategies on EigenLayer can wipe out nodeDelegator asset balance after max approving them to spend uint(256).max amount of asset

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

NodeDelegator have a function maxApproveToEigenStrategyManager(address asset) for approving a strategy to spend type(uint256).max amount of asset it's balance. Doing so on a custom strategy exposes Nodedelegator asset balance to a possible wiping.

Proof of Concept

After digging through EigenLayer strategy contracts i contacted the dev team on discord , and they confirmed to me that allowing operators to create their own custom strategy contracts are planned and are in internal discussion stages, this means in a near future operator can create their own custom strategy on eigenLayer. See discussion. With this in mind an operator can create his own strategy contract and program it to transfer out all the nodedelegator balances. This is possible because of the max approval given to the strategy. Since Kelp also have plans for supporting multiple strategies in the future, the max approval is a big issue on it's own in this context.

Tools Used

Manual review

Recommended Mitigation Steps

Since nodedelegator will always manually be called to deposit into strategy by kelp LRTManager, just approve the necessary amount before each deposit to avoid any funds loss in the future.

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