Swaps that include referrals cannot be executed when the defi plugin is deactivated

Summary

The protocol pays referrers every time a referred by them account generates a fee. The referral data is included in DefiSwap and is used to call increaseClaimableBy on the defi plugin. However, the defi plugin can be deactivated. Then all swaps including a referral information will be reverted.

Vulnerability Detail

To execute a swap, a wallet tx data will be offered by the Telcoin protocol to the onwer of the wallet. The wallet will sign the message offchain and the data will be used by an address having the SWAPPER role to execute the swap by calling AmirX.

When the fee is distributed, the defi plugin is called to increase the allowance of the referrer. However, if the defi plugin is deactivated, the whole tx will revert and any swaps with referrers will not be executed successfully.

This is exactly what is mentioned in the docs for not acceptable risks of pausing of external protocols. if a protocol prevents issues greater than the service they provide

In this case the protocol (the plugin) provides referrals feature, but when it gets deactivated swaps do not work as well. The Telcoin protocol may propose a new data to be signed where the referrer is removed, but then the referrer will lose their fee forever.

        if (defi.referrer != address(0) && defi.referralFee != 0) {
            ...
            require(
                defi.plugin.increaseClaimableBy(
                    defi.referrer,
                    defi.referralFee
                ),
                "AmirX: balance was not adjusted"
            );
        }

Impact

DoS of any swaps using a referrer.
Lose of funds for the referrers if Telcoin decides to execute the swaps without referrer to avoid reverting.

Code Snippet

DefiSwap


    struct DefiSwap {
        // Address of the swap aggregator or router
        address aggregator;
        // Plugin for handling referral fees
        ISimplePlugin plugin;
        // Token collected as fees
        ERC20 feeToken;
        // Address to receive referral fees
        address referrer;
        // Amount of referral fee
        uint256 referralFee;
        // Data for wallet interaction, if any
        bytes walletData;
        // Data for performing the swap, if any
        bytes swapData;
    }

increaseClaimableBy()

    function increaseClaimableBy(
        address account,
        uint256 amount
    ) external whenNotDeactivated onlyIncreaser returns (bool) {
        // if amount is zero do nothing
        if (amount == 0) {
            return false;
        }

        // keep track of old claimable and new claimable
        uint256 oldClaimable = _claimable[account].latest();
        uint256 newClaimable = oldClaimable + amount;

        // update _claimable[account] with newClaimable
        _claimable[account].push(newClaimable);

        // update _totalOwed
        _totalOwed += amount;

        // transfer TEL
        tel.safeTransferFrom(msg.sender, address(this), amount);

        emit ClaimableIncreased(account, oldClaimable, newClaimable);

        return true;
    }

AmirX._feeDispersal()

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

        // distribute reward
        if (defi.referrer != address(0) && defi.referralFee != 0) {
            TELCOIN.safeTransferFrom(safe, address(this), defi.referralFee);
            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(safe, TELCOIN.balanceOf(address(this)));
    }

Tool used

Manual Review

Recommendation

Add a mapping in AmirX which tracks unclaimed referral fees and totalUnclaimed which tracks the total amount of unclaimed fees.
Before calling defiPlugin.increaseClaimableBy(), check if the plugin is deactivated and if it's:
- active -> do the same as now
- inactive -> do not call the plugin, but increase the mapping value and totalUnclaimed. Then account for the total unclaimed when returning TELCOIN to the safe.
Introduce a function which allows the referrer to call increaseClaimableBy() when the plugin gets activated again.

sherlock-audit / 2024-02-telcoin-platform-audit-update-judging

ZdravkoHr. - Swaps that include referrals cannot be executed when the defi plugin is deactivated #54