bigbick123456789000 - Risk of transaction revert in `batchTelcoin` function since the contract doesn't approve spending of Telcoin tokens

[sherlock-admin]

bigbick123456789000

medium

Risk of transaction revert in batchTelcoin function since the contract doesn't approve spending of Telcoin tokens

Summary

The batchTelcoin function in the TelcoinDistributor contract lacks an explicit check for token approval before attempting to transfer Telcoin tokens. This may lead to unexpected behavior if the contract has not been approved to spend the required Telcoin amount. This issue can result in transaction reverts and potential disruptions in the intended workflow.

Vulnerability Detail

The batchTelcoin function is designed to transfer Telcoin tokens in batches to specified destinations. However, it assumes that the contract has been pre-approved to spend the necessary amount of Telcoin. Without an explicit check for approval, if the contract lacks the required allowance, the safeTransferFrom operation will fail, resulting in a transaction revert.

function batchTelcoin(
    uint256 totalWithdrawal,
    address[] memory destinations,
    uint256[] memory amounts
) internal {
    // ... (omitted for brevity)

    // transfers amounts
    TELCOIN.safeTransferFrom(owner(), address(this), totalWithdrawal);

    // ... (omitted for brevity)

    // Balance Check
    require(
        TELCOIN.balanceOf(address(this)) == initialBalance,
        "TelcoinDistributor: must not have leftovers"
    );
}

Impact

The batchTelcoin function may consistently revert, potentially leading to a Denial-of-Service (DoS) scenario. This situation arises when the contract lacks explicit checks for token approval, causing the safeTransferFrom operation to fail due to insufficient allowances. As a result, any attempt to execute the batchTelcoin function without proper approval may consistently result in transaction reverts, leading to a DoS condition where the intended functionality of the contract is disrupted.

Code Snippet

Link

Tool used

Manual Review

Recommendation

Introduce an explicit approval check within the batchTelcoin function. This check ensures that the contract has the necessary allowance to spend Telcoin tokens before attempting transfers.

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid because { i consider this invalid because this is an issue between owner and the contract; the owner must have approved the contract to be spending the telecoin token; if it wasnt done behind the scene; the issue should be valid}

[nevillehuang]

Invalid, batchTelcoin() is only called within executeTransaction(), wherein approval will have been supplied externally by owner