sherlock-audit / 2024-11-telcoin-judging

0 stars 0 forks source link

hunter_w3b - Insufficient TELCOIN Balance Check in `_feeDispersal` Causes Potential Transaction Revert #148

Open sherlock-admin3 opened 2 weeks ago

sherlock-admin3 commented 2 weeks ago

hunter_w3b

Medium

Insufficient TELCOIN Balance Check in _feeDispersal Causes Potential Transaction Revert

Summary

A missing balance check in the _feeDispersal function may lead to transaction failures if defi.referralFee is greater than the contract’s available TELCOIN balance. As a result, users could experience unexpected reverts when attempting a swap with a referral fee set, causing failed transactions.

Root Cause

The _feeDispersal function lacks a check to verify that the contract holds enough TELCOIN for the referral fee before attempting the transfer. This leads to a potential revert if the balance is insufficient.

    function _feeDispersal(DefiSwap memory defi) internal {
        // must buy into TEL
        if (defi.feeToken != TELCOIN)
            _buyBack(
                defi.feeToken,
                defi.aggregator,
                defi.defiSafe,
                defi.swapData
            );

        // distribute reward
        if (defi.referrer != address(0) && defi.referralFee != 0) {
            TELCOIN.forceApprove(address(defi.plugin), 0);
            TELCOIN.safeIncreaseAllowance(
                address(defi.plugin),
                defi.referralFee
            );
            require(
                defi.plugin.increaseClaimableBy(
                    defi.referrer,
                    defi.referralFee
                ),
                "AmirX: balance was not adjusted"
            );
        }
        // retain remainder
        if (TELCOIN.balanceOf(address(this)) > 0)
            TELCOIN.safeTransfer(
                defi.defiSafe,
                TELCOIN.balanceOf(address(this))
            );
    }

https://github.com/sherlock-audit/2024-11-telcoin/blob/main/telcoin-audit/contracts/swap/AmirX.sol#L183-L214

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

  1. User calls defiSwap with a referral fee set in defi.referralFee.
  2. _buyBack is completed, converting other tokens to TELCOIN.
  3. _feeDispersal attempts to allocate the referral fee without confirming the TELCOIN balance.
  4. If the TELCOIN balance is insufficient, the transaction reverts, leading to a failed swap.

Impact

The user experiences transaction reversion when attempting to complete a swap with a referral fee. This limits the contract’s usability, as users cannot reliably initiate swaps with referral fees when the balance is insufficient.

PoC

No response

Mitigation

To mitigate the insufficient balance issue in _feeDispersal for handling referral fees, you can update the function to include a safeTransferFrom.

if (defi.referrer != address(0) && defi.referralFee != 0) {

+    // Transfer the referral fee from the safe to the contract
+    TELCOIN.safeTransferFrom(defi.defiSafe, address(this), defi.referralFee);

    // Set allowance for the plugin to handle the referral fee
    TELCOIN.forceApprove(address(defi.plugin), 0);
    TELCOIN.safeIncreaseAllowance(address(defi.plugin), defi.referralFee);

    // Call the plugin function to increase the claimable balance for the referrer
    require(
        defi.plugin.increaseClaimableBy(defi.referrer, defi.referralFee),
        "AmirX: Balance adjustment for referrer failed"
    );
}
  1. Transfer: The safeTransferFrom function transfers the referral fee from defi.defiSafe to the contract itself.