fix(contracts): Frontrun Permit in GasSwap #30
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose or design rationale of this PR
Describe your change. Make sure to answer these three questions: What does this PR do? Why does it do it? How does it do it?
This PR fix the following issue
In brief, users may face front-running vulnerabilities when invoking
swap(...)inGasSwap.sol, as an attacker could executepermitinERC20Permit.solbefore userswaptx is mined, leading to the user's transaction reverting due to the signature being already used.According to https://github.com/Amxx/openzeppelin-contracts/blob/487b3f9cdd0561e151d4965ca18590bba907209a/contracts/token/ERC20/extensions/IERC20Permit.sol#L16-L33, doesn't use
safePermitdoesn't mean users are protected from front-running attacks.details:
In
ERC20Permit.solpermitfunction: https://github.com/Amxx/openzeppelin-contracts/blob/487b3f9cdd0561e151d4965ca18590bba907209a/contracts/token/ERC20/extensions/ERC20Permit.sol#L44-L67when user sign
bytes32 hash, inbytes32 structHash, one of the signed data is_useNonce(owner)so let's see what's
_useNonce(address)do.https://github.com/Amxx/openzeppelin-contracts/blob/487b3f9cdd0561e151d4965ca18590bba907209a/contracts/utils/Nonces.sol#L27-L34
so every time
_useNonce(...)is triggered user nonce will increase by 1, what does this mean?every time
function permit(...)is called user nonce will increase by 1.so here is the scenario,
_nonces[A] = 0and callfunction swap(...)ingasSwap.solfunction permit(...), after attacker execute permit_nonces[A] = 1_nonces[A] = 0, but nowERC20Permit.solexpect user to sign hash at_nonces[A] = 1, so the transaction will always revert.