Closed c4-submissions closed 10 months ago
raymondfam marked the issue as insufficient quality report
M-01 from the bot.
raymondfam marked the issue as primary issue
fatherGoose1 marked the issue as unsatisfactory: Invalid
fatherGoose1 marked the issue as unsatisfactory: Invalid
Lines of code
https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L136 https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L194 https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/NodeDelegator.sol#L86
Vulnerability details
Impact
For these non-erc20 compliant assets (e.g., USDT) which don't have bool return in the transfer()/transferFrom(), the assets transfer always reverts.
Proof of Concept
For most LST assets, they are erc20 compliant and there's no problem. Because owner can add new asset, in case the asset doesn't have a bool return in its transfer()/transferFrom(), the token transfer from/to the protocol always reverts and the asset can't work in the protocol.
Tools Used
Recommended Mitigation Steps
Use the safe version of erc20 transfer()/transferFrom(), i.e., safeTransfer()/safeTransferFrom().
Assessed type
Token-Transfer