Diana - Approve function is subject to front-run attack

[sherlock-admin]

Diana

medium

Approve function is subject to front-run attack

Summary

Approve function in FootiumEscrow is subject to front-run attack because the approve method overwrites the current allowance regardless of whether the spender already used it or not. In case the spender spent the amount, the approve function will approve a new amount.

Vulnerability Detail

The approve method overwrites the current allowance regardless of whether the spender has already used it or not. It allows the spender to front-run and spend the amount before the new allowance is set.

Scenario:

• Alice allows Bob to transfer N of Alice's tokens (N>0) by calling the erc20Contract.approve method, passing Bob's address and N as the method arguments
• After some time, Alice decides to change from N to M (M>0) the number of Alice's tokens Bob is allowed to transfer, so she calls the erc20Contract.approve method again, this time passing Bob's address and M as the method arguments
• Bob notices Alice's second transaction before it was mined and quickly sends another transaction that calls the erc20Contract.transfer method to transfer N Alice's tokens somewhere
• If Bob's transaction is executed before Alice's transaction, then Bob will successfully transfer N Alice's tokens and will gain the ability to transfer another M tokens
• Before Alice notices that something went wrong, Bob calls the erc20Contract.transfer method again, this time to transfer M Alice's tokens.
• So, Alice's attempt to change Bob's allowance from N to M (N>0 and M>0) made it possible for Bob to transfer N+M of Alice's tokens, while Alice never wanted to allow so many of her tokens to be transferred by Bob.

Impact

The user whose address is passed as the to parameter in erc20Contract.approve(to, amount) while calling setApprovalForERC20 can abuse this by monitoring the mem-pool and spend more amount of tokens than allowed.

Code Snippet

https://github.com/sherlock-audit/2023-04-footium/blob/main/footium-eth-shareable/contracts/FootiumEscrow.sol#L80

    function setApprovalForERC20(
        IERC20 erc20Contract,
        address to,
        uint256 amount
    ) external onlyClubOwner {
        erc20Contract.approve(to, amount);
    }

Tool used

Manual Review

Recommendation

Use increaseAllowance and decreaseAllowance instead of approve as OpenZeppelin ERC20 implementation. Please see the details here:
https://forum.openzeppelin.com/t/explain-the-practical-use-of-increaseallowance-and-decreaseallowance-functions-on-erc20/15103/4

Duplicate of #8