-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Frontrunning Approve Race Condition #145
Comments
raymondfam marked the issue as primary issue |
raymondfam marked the issue as sufficient quality report |
raymondfam marked the issue as high quality report |
hieronx marked the issue as disagree with severity |
|
hieronx (sponsor) acknowledged |
gzeon-c4 marked the issue as unsatisfactory: |
gzeon-c4 marked the issue as unsatisfactory: |
1 similar comment
gzeon-c4 marked the issue as unsatisfactory: |
Lines of code
https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/token/ERC20.sol#L131
https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/token/ERC20.sol#L139
https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/token/ERC20.sol#L148
Vulnerability details
Vulnerability Details
There are no checks in the already approved spender function, which may lead to frontrunning.
Ethereum mempool is the place where all pending transactions sit until miner decided to include them into the block. For the most time, transactions with the highest gas prices are included first as the miners get the gas price for including them into the block.
Some have found a way to exploit this miners behaviour. Front-running, works like this: Transaction A is broadcasted with a higher gas price than an already pending transaction B so that A gets mined before B.
Exploit Scenario
If Bob monitors the mempool, he could submit his transaction with a higher gas cost so it would be executed before Alice's TX.
Attack is also possible because approve() overrides current allowance. It doesn't increase/decrease allowance.
Impact
Unsafe ERC20 approve that do not handle non-standard erc20 behavior.
1.Some token contracts do not return any value.
2.Some token contracts revert the transaction when the allowance is not zero.
Proof of Concept
Tools Used
Manual Review
Recommended Mitigation Steps
First: To address this issue, Alice must initiate two transactions. First, she sets the allowance to 0, effectively resetting it for Bob. The second transaction is required to approve the new desired allowance.
Second: Employ non-standard functions provided by ERC20, namely increaseAllowance() and decreaseAllowance(). So if the Bob was approved before, and the approved amount is higher/lower, call the increaseAllowance() or decreaseAllowance(). These functions are integral components of the widely adopted OpenZeppelin ERC20 implementation but the problem is it's possible to have human error calling the approve function instead of increaseAllowance() and decreaseAllowance().
Assessed type
ERC20
The text was updated successfully, but these errors were encountered: