Kelp DAO | rsETH - alexfilippov314's results

Lines of code

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

Vulnerability details

The NodeDelegator.transferBackToLRTDepositPool function doesn't check that LRT_DEPOSIT_POOL was set: LRTConfig.sol

function getContract(bytes32 contractKey) external view override returns (address) {
    return contractMap[contractKey];
}

NodeDelegator.sol

function transferBackToLRTDepositPool(
    address asset,
    uint256 amount
)
    external
    whenNotPaused
    nonReentrant
    onlySupportedAsset(asset)
    onlyLRTManager
{
    address lrtDepositPool = lrtConfig.getContract(LRTConstants.LRT_DEPOSIT_POOL);

    if (!IERC20(asset).transfer(lrtDepositPool, amount)) {
        revert TokenTransferFailed();
    }
}

In the case when this address was not set the funds will be transfered to the zero address and will be lost for good.

The probability of this situation is low since in most cases the LRTOracle.getRSETHPrice also uses this address and therefore the LRTDepostPool.depositAsset transaction will be reverted. But in the case when rsEthSupply == 0 the LRTOracle.getRSETHPrice doesn't use LRT_DEPOSIT_POOL. In this case, the flow LRTDepostPool.depositAsset -> LRTDepostPool.transferAssetToNodeDelegator -> NodeDelegator.transferBackToLRTDepositPool can be executed without reverts and funds can be lost.

Impact

Funds can be lost in the NodeDelegator.transferBackToLRTDepositPool if the LRT_DEPOSIT_POOL contract address was not set in the LRTConfig. The probability of such a situation is low but it is better to prevent it completely.

Proof of Concept

-

Tools Used

Manual Review

Recommended Mitigation Steps

Consider adding a check that lrtDepositPool is not equal to the zero address in the NodeDelegator.transferBackToLRTDepositPool.

Assessed type

Invalid Validation