pontifex - FootiumEscrow_Don't use approve() to decrease/increase allowance

[sherlock-admin]

pontifex

medium

FootiumEscrow_Don't use approve() to decrease/increase allowance

Summary

The setApprovalForERC20 function sets amount as an allowance for to address to spend the escrow contract's erc20Contract tokens. This function allows the club owner to call the ERC20.approve function. In case when the club owner wants to change the preset allowance it is recommended to call ERC20.increaseAllowance or ERC20.decreaseAllowance instead of ERC20.approve function.

Vulnerability Detail

Suppose the club owner preset allowance M > 0 for address(attacker) (tx1) and wants to change allowance to N > 0 (tx2). Attacker intercepts tx2, and makes tx1.5 (transferFrom(M)). Then the attacker sends tx3 (transferFrom(N)). The attacker spent M + N tokens from the escrow contract.

Impact

The club owner takes a risk of tokens loss when changes allowances through the ERC20.approve function in one transaction. In other case it is necessary to make two transactions to firstly reset allowances with approve(0).

Code Snippet

https://github.com/sherlock-audit/2023-04-footium/blob/11736f3f7f7efa88cb99ee98b04b85a46621347c/footium-eth-shareable/contracts/FootiumEscrow.sol#L75-L81

Tool used

Manual Review

Recommendation

It is recommended adding functions to call ERC20.increaseAllowance or ERC20.decreaseAllowance, or using “expected_current_allowance” as an argument.

Duplicate of #8