0xStalin - Spender is allowed to transfer more tokens than the owner of the tokens ever wanted to allow the spender to spend

[sherlock-admin]

0xStalin

high

Spender is allowed to transfer more tokens than the owner of the tokens ever wanted to allow the spender to spend

Summary

The setApprovalForERC20() uses the approve() of the ERC20 standard and doesn't reset the total allowance to 0 before updating the new value.
A malicious user can take advantage of this and perform a transaction order to end up spending more tokens than the total tokens the owner intended to allow this user to spend, a more detailed example is in the "Vulnerability Detail" Section.

You can refer to this thread about this transaction ordering caused by the approve() of the EIP20 proposal

• ERC: Token standard ethereum/EIPs#20 (comment)

Vulnerability Detail

Here is a possible attack scenario:

1. Alice allows Bob to transfer N of Alice's tokens (N>0) by calling the approve method on a Token smart contract, passing the Bob's address and N as the method arguments
2. 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 approve method again, this time passing the Bob's address and M as the method arguments
3. Bob notices the Alice's second transaction before it was mined and quickly sends another transaction that calls the transferFrom method to transfer N Alice's tokens somewhere
4. If the Bob's transaction will be executed before the Alice's transaction, then Bob will successfully transfer N Alice's tokens and will gain an ability to transfer another M tokens
5. Before Alice noticed that something went wrong, Bob calls the transferFrom method again, this time to transfer M Alice's tokens.
6. So, an Alice's attempt to change the 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

• A spender can abuse and transfer more tokens than the owner of the tokens ever wanted to allow the spender to spend by performing a transaction ordering attack.

Code Snippet

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

Tool used

Manual Review

Recommendation

first reduce the spender's allowance to 0 and set the desired value afterwards. Additionally make sure to check the returned value to ensure that the allowance was indeed updated as expected

function setApprovalForERC20(
    IERC20 erc20Contract,
    address to,
    uint256 amount
) external onlyClubOwner {
+   require(erc20Contract.approve(to, 0),"Error while reseting the approval amount to 0");
    require(erc20Contract.approve(to, amount),"Error while granting the desired allowance to the spender");
}

Duplicate of #8