Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid the use of increaseAllowance and decreaseAllowance from ERC20 that are recently deprecated. #498

Closed
c4-submissions opened this issue Sep 14, 2023 · 3 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-320 low quality report This report is of especially low quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-submissions
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/token/ERC20.sol#L139-L160

Vulnerability details

Impact

The ERC20.sol file is an implementation of the ERC20 standard that is to be used in the codebase, but the increaseAllowance and decreaseAllowance of ERC20 OZ library has been recently deprecated.
The increaseAllowance and decreaseAllowance were non-standard with EIP-20 and the discussion to make remove it was finally merged recently.

In this codebase, the ERC20.sol is expected to work with the ERC-4626 vault that are the LPs in which the assets are ERC20 tokens. Using deprecated codebase can later prove to be problematic and even cause issues while trying to use assets following the latest standard.

Impact: Medium, as functionality is not working as expected but without a value loss.

Likelihood: Medium, as multiple methods are not compliant with the standard.

Proof of Concept

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/token/ERC20.sol#L139C5-L146C6

//In File::src/token/ERC20.sol

function increaseAllowance(address spender, uint256 addedValue) external returns (bool) {
        uint256 newValue = allowance[_msgSender()][spender] + addedValue;
        allowance[_msgSender()][spender] = newValue;

        emit Approval(_msgSender(), spender, newValue);

        return true;
    }

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/token/ERC20.sol#L148C2-L159C6

//In File::src/token/ERC20.sol

   function decreaseAllowance(address spender, uint256 subtractedValue) external returns (bool) {
        uint256 allowed = allowance[_msgSender()][spender];
        require(allowed >= subtractedValue, "ERC20/insufficient-allowance");
        unchecked {
            allowed = allowed - subtractedValue;
        }
        allowance[_msgSender()][spender] = allowed;

        emit Approval(_msgSender(), spender, allowed);

        return true;
    }

Tools Used

Manual review

Recommended Mitigation Steps

Avoid the use of increaseAllowance and decreaseAllowance or use SafeERC20 library which is still available.

Assessed type

ERC20

@c4-submissions c4-submissions added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Sep 14, 2023
c4-submissions added a commit that referenced this issue Sep 14, 2023
@c4-pre-sort c4-pre-sort added the low quality report This report is of especially low quality label Sep 15, 2023
@c4-pre-sort
Copy link

raymondfam marked the issue as low quality report

@c4-pre-sort
Copy link

raymondfam marked the issue as duplicate of #320

@c4-judge
Copy link

gzeon-c4 marked the issue as unsatisfactory:
Out of scope

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-320 low quality report This report is of especially low quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

3 participants