Approve function of ERC20.sol is prone to race condition vulnerability

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/token/ERC20.sol#L131-L159

Vulnerability details

Impact

The race condition that happens most frequently on the network today is the race condition in the ERC20 token standard.
The ERC20.sol includes a function called 'approve', which allows an address to approve another address to spend tokens on their behalf.

Assume that Alice has approved Eve to spend n of her tokens, then Alice decides to change Eve's approval to m tokens.
Alice submits a function call to approve with the value n for Eve. Eve runs an Ethereum node, so she knows that Alice is going to change her approval to m.
Eve then submits a transferFrom request, sending n of Alice's tokens to herself, but gives it a much higher gas price than Alice's transaction.
The transferFrom executes first so gives Eve n tokens and sets Eve's approval to zero. Then Alice's transaction executes and sets Eve's approval to m.
Eve then sends those m tokens to herself as well. Thus, Eve gets n + m tokens, even though she should have gotten at most max(n,m)

Proof of Concept

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/token/ERC20.sol#L131-L159

Tools Used

VSCode

Recommended Mitigation Steps

The best fix for the ERC20 race condition is to add a field to the inputs of approve, which is the expected current value,
and to have approve revert if Eve's current allowance is not what Alice indicated she was expecting.
Also, we can add a safe approve function.

From the user's perspective, it is possible to mitigate the ERC20 race condition by setting approvals to zero before changing them.

Assessed type

Token-Transfer

[c4-pre-sort]

raymondfam marked the issue as duplicate of #145

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory:
Invalid

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory:
Invalid