sherlock-audit / 2023-10-aloe-judging

9 stars 6 forks source link

alymurtazamemon - Race condition in the ERC20 `approve` function may lead to token theft #57

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

alymurtazamemon

medium

Race condition in the ERC20 approve function may lead to token theft

Summary

The approve function permits other users to spend tokens on behalf of the token owner. However, it has been demonstrated to be vulnerable to frontrunning attacks. In this scenario, when the owner updates (decreases) the allowance granted to other users, these users can frontrun the owner's transaction, allowing them to transfer the previous amount of tokens allowed and then get additional tokens once the owner's transaction is completed.

Vulnerability Detail

A known race condition in the ERC20 standard, on the approve function, could lead to the theft of tokens.

The ERC20 standard describes how to create generic token contracts. Among others, an ERC20 contract defines these two functions:

These functions give permission to a third party to spend tokens. Once the function approve(spender, value) has been called by a user, spender can spend up to value of the user’s tokens by calling transferFrom(user, to, value).

This schema is vulnerable to a race condition when the user calls approve a second time on a spender that has already been allowed. If the spender sees the transaction containing the call before it has been mined, then the spender can call transferFrom to transfer the previous value and still receive the authorization to transfer the new value.

Exploit Scenario

  1. Alice calls approve(Bob, 1000). This allows Bob to spend 1000 tokens.
  2. Alice changes her mind and calls approve(Bob, 500). Once mined, this will decrease to 500 the number of tokens that Bob can spend.
  3. Bob sees Alice’s second transaction and calls transferFrom(Alice, X, 1000) before approve(Bob, 500) has been mined.
  4. If Bob’s transaction is mined before Alice’s, 1000 tokens will be transferred by Bob. Once Alice’s transaction is mined, Bob can call transferFrom(Alice, X, 500). Bob has transferred 1500 tokens, contrary to Alice’s intention.

Impact

Users will lose more tokens than they want to approve to other users.

Code Snippet

function approve(address spender, uint256 shares) external returns (bool) {
    allowance[msg.sender][spender] = shares;

    emit Approval(msg.sender, spender, shares);

    return true;
}

Lender.sol - Lines 321 - 327

Tool used

Manual Review, IERC20.sol documentation

Recommendation

While this issue is known and can have a severe impact, there is no straightforward solution.

One workaround is to use two non-ERC20 functions allowing a user to increase and decrease the approve (see increaseApproval and decreaseApproval of StandardToken.sol#L63-L98).

sherlock-admin2 commented 1 year ago

2 comment(s) were left on this issue during the judging contest.

tsvetanovv commented:

Low

MohammedRizwan commented:

low severity as openzeppelin has removed the increaseAllowance() and decreaseAllowance() functions.