code-423n4 / 2023-10-wildcat-findings

12 stars 9 forks source link

The `approve` function can be frontrun #726

Closed c4-submissions closed 10 months ago

c4-submissions commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketToken.sol#L31-L34 https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/market/WildcatMarketToken.sol#L41-L57

Vulnerability details

Impact

Bob steals tokens from Alice.

Proof of Concept

In the file WildcatMarketToken.sol there is an approve function:

  function approve(address spender, uint256 amount) external virtual nonReentrant returns (bool) {
    _approve(msg.sender, spender, amount);
    return true;
  }

that is susceptible to a frontrunning attack. Let's suppose Alice calls the approve function with spender == address(Bob) and amount == 100. Soon after that Alice decides that her allowance to Bob is too big and decides to reduce it - Alice calls the approve function again with spender == address(Bob) and reduced amount amount == 50. Bob monitors the Ethereum mempool and sees that Alice gave him some allowance and then reduced it. Bob is not happy with that. Bob waits for Alice's first transaction with amount == 100 to pass. After this transaction is complete Bob has an allowance of amount == 100. Right after that Bob calls transferFrom function from the file WildcatMarketToken.sol frontrunning Alice's second transaction with amount == 50. As a result Bob transfers amount == 100 to his address. After that Alice's second transaction with amount == 50 is completed and Bob is allowed to spend 50 more units of a token. Bob calls transferFrom again and takes these 50 units of a token as well. As a result, Alice intended to give Bob an allowance of 50 units of a token, but Bob managed to take from Alice 150 units of a token, essentially stealing 100 units of a token.

Tools Used

Manual review.

Recommended Mitigation Steps

Use OZ libraries.

Assessed type

MEV

d1ll0n commented 10 months ago

This is a generic ERC20 problem that's only relevant when you give approval to untrusted parties or contracts that allow untrusted parties to trigger a transferFrom. Definitely not high severity.

c4-pre-sort commented 10 months ago

minhquanym marked the issue as primary issue

c4-pre-sort commented 10 months ago

minhquanym marked the issue as low quality report

c4-sponsor commented 10 months ago

laurenceday (sponsor) disputed

c4-sponsor commented 10 months ago

laurenceday marked the issue as disagree with severity

c4-judge commented 10 months ago

MarioPoneder marked the issue as unsatisfactory: Overinflated severity