[ M ] Some ERC20 tokens return a false bool even if transfer was successful

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-08-sparkn/blob/0f139b2dc53905700dd29a01451b330f829653e9/src/Distributor.sol#L116

Summary

Some particularly pathological ERC20 tokens (e.g. Tether Gold) declare a bool return, but then return false, even when the transfer was successful.

Vulnerability Details

If Tether Gold or any other token with similar behavior is used, the safeTransfer method might not be able to correctly determine if the transfer was successful.

Impact

As a result, the state of the contract might not be updated appropriately, and funds could be left in the contract, leading to potential disputes or incorrect handling of prizes.

Tools Used

Manual Review

Recommendations

By comparing the initial and final token balances of the recipient after the safeTransfer call, you can determine if the transfer was successful.

Something like this:

    // Record initial balance
        uint256 initialBalance = erc20.balanceOf(winners[i]);

        // Transfer tokens
        erc20.safeTransfer(winners[i], amount);

        // Record final balance
        uint256 finalBalance = erc20.balanceOf(winners[i]);

        // Check if transfer was successful
        if (finalBalance != initialBalance + amount) {
            revert Distributor__TransferFailed();
        }
    }

Cyfrin / 2023-08-sparkn