0xmystery - ERC20 Approve/Allowance Front-Running Vulnerability and Mitigation in GSPVault.sol with USDT Example

[sherlock-admin2]

0xmystery

medium

ERC20 Approve/Allowance Front-Running Vulnerability and Mitigation in GSPVault.sol with USDT Example

Summary

This report delves into a potential security vulnerability in the ERC20 token standard's approve function, as implemented in many smart contracts. It particularly focuses on how certain tokens, like USDT (Tether), have adopted a security measure to mitigate this risk.

Vulnerability Detail

The vulnerability in question involves the standard approve function of ERC20 tokens, which could be exploited through a front-running attack. This occurs when an owner attempts to change the allowance of a spender and the spender, by monitoring the pending transactions, uses the old allowance before the new one is set.

Let's break down the scenario:

1. Initial State: An owner (A) has allowed a spender (B) to spend 1000 tokens on their behalf.
2. Owner's Intention: A wants to reduce this allowance to 500 tokens.
3. Front-Running Scenario: Before A's transaction to reduce the allowance is confirmed, B notices this pending transaction.
4. Exploitation: B quickly sends a transaction to transfer 1000 tokens (the full amount of the current allowance) via transferFrom() below and gets it confirmed before A's transaction.

https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPVault.sol#L250-L263

    function transferFrom(
        address from,
        address to,
        uint256 amount
    ) public returns (bool) {
        require(amount <= _SHARES_[from], "BALANCE_NOT_ENOUGH");
        require(amount <= _ALLOWED_[from][msg.sender], "ALLOWANCE_NOT_ENOUGH");

        _SHARES_[from] = _SHARES_[from] - amount;
        _SHARES_[to] = _SHARES_[to] + amount;
        _ALLOWED_[from][msg.sender] = _ALLOWED_[from][msg.sender] - amount;
        emit Transfer(from, to, amount);
        return true;
    }

6. Final Result: B can then use the newly set allowance of 500 tokens, effectively getting access to a total of 1500 tokens, more than what A intended.

Impact

Exploitation can lead to unauthorized use of tokens, posing a significant risk of financial loss. This risk is heightened in contracts where large allowances are frequently adjusted.

Code Snippet

Standard ERC20 approve function implementation as reflected in GSPVault.sol :

https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPVault.sol#L265-L282

    /**
     * @dev Approve the passed address to spend the specified amount of tokens on behalf of msg.sender.
     * @param spender The address which will spend the funds.
     * @param amount The amount of tokens to be spent.
     */
    function approve(address spender, uint256 amount) public returns (bool) {
        _approve(msg.sender, spender, amount);
        return true;
    }

    function _approve(
        address owner,
        address spender,
        uint256 amount
    ) private {
        _ALLOWED_[owner][spender] = amount;
        emit Approval(owner, spender, amount);
    }

Mitigation Example: USDT Token

USDT (Tether) token contracts have implemented a modified approve mechanism as a security measure. In this approach, if an owner wants to change an existing, non-zero allowance for a spender, they must first set the allowance to zero. Only then can they set it to a new value. This enforced two-step process is crucial in mitigating the risk of front-running attacks.

Modified approve function in USDT:

function approve(address spender, uint256 amount) public returns (bool) {
    require((amount == 0) || (_ALLOWED_[msg.sender][spender] == 0),
        "Reset allowance to zero before updating it");
    _approve(msg.sender, spender, amount);
    return true;
}

Tool used

Manual Review

Recommendation

For ERC20 token contracts, it is recommended to adopt a similar two-step process for changing allowances, either by enforcing it in the contract logic (as seen in USDT) or by educating users to manually set allowances to zero before changing them. This practice adds an important layer of security, despite the trade-off in user convenience and the need for increased transaction fees due to the additional transaction.

Additionally, developers and users should be made aware of these practices to ensure compatibility and avoid failed transactions. This approach represents a balanced solution that emphasizes the protection of user assets, particularly in contracts with significant transaction volumes or high-value assets.

Duplicate of #103