approve function can be frontrun and funds will be stolen as a result

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

The user that gives allowances will lose funds.

Proof of Concept

In the file ERC20.sol there is a function approve. This function is problematic as it is susceptible to frontrunning attacks.

PoC:
Consider the following scenario: Alice calls the function approve (with Bob's address and value == 20) and approves Bob to spend 20 units of a token on her behalf. Shortly after that Alice decides to decrease Bob's allowance to 10 units of a token by calling again the function approve (with Bob's address and value == 10). At the end, Alice expects that Bob will be able to spend 10 units of a token on her behalf. Bob carefully monitors the mempool and sees Alice's two transactions - the one that sets Bob's allowance to 20 and the one that sets Bob's allowance to 10. Bob waits the first transaction to be executed and the state to be updated. After the first transaction is completed, Bob is allowed to use 20 units of a token on Alice's behalf. Bob frontruns the second Alice's transaction calling the function transferFrom (from the file ERC20.sol) and sending the 20 units to his address. The second Alice's transaction is executed after that and Bob is granted to use 10 more units of a token on Alice's behalf. Bob calls the transferFrom function again and transfers 10 more units of a token to his address. As a result Bob managed to take 30 units of a token from Alice, while Alice expects that Bob can control only 10 units of her tokens. Thus, Bob manages to steal 20 units of a token from Alice.

Tools Used

Manual review

Recommended Mitigation Steps

Remove the approve function and leave only the functions increaseAllowance and decreaseAllowance.

Assessed type

MEV

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #145

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory:
Invalid

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory:
Invalid