code-423n4 / 2023-11-kelp-findings

13 stars 11 forks source link

Inadequate Asset Balance Check in `transferAssetToNodeDelegator` Function #147

Closed c4-submissions closed 11 months ago

c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/c5fdc2e62c5e1d78769f44d6e34a6fb9e40c00f0/src/LRTDepositPool.sol#L183-L197

Vulnerability details

Impact

The lack of an asset balance check in this function could result in failed asset transfers, disrupting the intended functionality of the contract. Users relying on this function might experience unexpected failures, affecting the reliability and trustworthiness of the smart contract.

Proof of Concept

In the transferAssetToNodeDelegator function, there is a missing check to verify whether the contract holds an adequate balance of the specified asset before attempting the transfer. Without this check, the contract may attempt to transfer more assets than it possesses, resulting in a failed transaction and potential disruptions to the intended functionality of the contract.

function transferAssetToNodeDelegator(
    uint256 ndcIndex,
    address asset,
    uint256 amount
)
    external
    nonReentrant
    onlyLRTManager
    onlySupportedAsset(asset)
{
    address nodeDelegator = nodeDelegatorQueue[ndcIndex];

    // Check if the contract has enough assets to transfer
    uint256 contractBalance = IERC20(asset).balanceOf(address(this));
    require(contractBalance >= amount, "Insufficient contract balance");

    // Perform the asset transfer
    if (!IERC20(asset).transfer(nodeDelegator, amount)) {
        revert("Token transfer failed");
    }
}

The purpose of this function is to transfer a specified amount of a given asset to a Node Delegator. However, the omission of the balance check (contractBalance >= amount) poses a significant risk. Without this check, the function assumes that the contract always has enough assets to transfer, which might not be the case, leading to potential failure scenarios.

Tools Used

Manual

Recommended Mitigation Steps

Ensure that the contract's balance of the specified asset is sufficient before initiating the transfer.

uint256 contractBalance = IERC20(asset).balanceOf(address(this));
require(contractBalance >= amount, "Insufficient contract balance");

Assessed type

Other

c4-pre-sort commented 11 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 11 months ago

raymondfam marked the issue as duplicate of #69

c4-judge commented 11 months ago

fatherGoose1 changed the severity to QA (Quality Assurance)

c4-judge commented 11 months ago

fatherGoose1 marked the issue as grade-b