`RdpxV2Core.sol`: `approveContractToSpend` should allow 0 values for tokens like CRV

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L403-L412

Vulnerability details

Impact

The function approveContractToSpend is used to approve a contract to spend a certain amount of tokens. There are some require statements in this function, to sanitize the input.

function approveContractToSpend(
    address _token,
    address _spender,
    uint256 _amount
  ) external onlyRole(DEFAULT_ADMIN_ROLE) {
    _validate(_token != address(0), 17);
    _validate(_spender != address(0), 17);
    _validate(_amount > 0, 17);
    IERC20WithBurn(_token).approve(_spender, _amount);
  }

As can be seen from the validate function call, the _amount passed must be non-zero.

This can cause an issue in a token like CRV. The project aims to provide liquidity on curve and stake those LP tokens to earn CRV tokens which will be used ot pay the premium according to the docs. CRV tokens have a special approve function which has built in race condition prevention and require users to approve a value of 0 before changing the allowance from one non-zero value to another. This can be seen in the approve functionof the CRV token written in vyper as shown below.

@external
def approve(_spender : address, _value : uint256) -> bool:
    assert _value == 0 or self.allowances[msg.sender][_spender] == 0
    self.allowances[msg.sender][_spender] = _value
    log Approval(msg.sender, _spender, _value)
    return True

So if the contract has some pending CRV allowance, it will be impossible to change that allowance since an allowance of 0 cannot be set.

Proof of Concept

The linked snippet above shows the live code in the CRV token.

Tools Used

Manual Review

Recommended Mitigation Steps

Allow 0 value approvals

Assessed type

DoS

[c4-pre-sort]

bytes032 marked the issue as duplicate of #797

[c4-pre-sort]

bytes032 marked the issue as sufficient quality report

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)