Use of the deprecated safeApprove() function of openzeppelin containing security vulnerabilities.

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/BackingManager.sol#L72
https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/BackingManager.sol#L73
https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/RevenueTrader.sol#L191
https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/RevenueTrader.sol#L192

Vulnerability details

Impact

The BackingManager::grantRTokenAllowance and the RevenurTrader::_distributeTokenToBuy functions make use of safeApprove() function.
The original ERC20 token approval function is susceptible to front-running attacks, where malicious actors can modify quotas after seeing a transaction, but before it is mined.
The safeApprove function inherited this vulnerability and added unnecessary complexity, making it an additional insecurity.
This is why both functions have been deprecated.

Proof of Concept

• SafeERC20.safeApprove() Has unnecessary and unsecure added behavior OpenZeppelin/openzeppelin-contracts#2219

• https://github.com/OpenZeppelin/openzeppelin-contracts/blob/566a774222707e424896c0c390a84dc3c13bdcb2/contracts/token/ERC20/utils/SafeERC20.sol#L38

• The grantRTokenAllowance function in theBackingManage contract:

    function grantRTokenAllowance(IERC20 erc20) external notFrozen {
        require(assetRegistry.isRegistered(erc20), "erc20 unregistered");
@>      IERC20(address(erc20)).safeApprove(address(rToken), 0);
@>      IERC20(address(erc20)).safeApprove(address(rToken), type(uint256).max);
    }

• The _distributeTokenToBuy function in the RevenueTrader contract:

  function _distributeTokenToBuy() internal {
        uint256 bal = tokenToBuy.balanceOf(address(this));

@>      tokenToBuy.safeApprove(address(distributor), 0);
@>      tokenToBuy.safeApprove(address(distributor), bal);

        distributor.distribute(tokenToBuy, bal);
    }

Tools Used

Manual anlysis.

Recommended Mitigation Steps

Instead of using safeApprove, OpenZeppelin introduced safeIncreaseAllowance and safeDecreaseAllowance functions in the SafeERC20 library, which allow you to safely increment or decrement the allowance without directly setting it to a specific value.

Assessed type

ERC20