sherlock-audit / 2024-04-teller-finance-judging

6 stars 6 forks source link

MaslarovK.eth - Did not approve to zero first. #262

Closed sherlock-admin4 closed 2 months ago

sherlock-admin4 commented 2 months ago

MaslarovK.eth

medium

Did not approve to zero first.

Summary

Allowance was not set to zero first before changing the allowance.

Vulnerability Detail

Some ERC20 tokens (like USDT) do not work when changing the allowance from an existing non-zero allowance value.

Impact

For example Tether (USDT)'s approve() function will revert if the current approval is not zero, to protect against front-running changes of approvals. However, if the token involved is an ERC20 token that does not work when changing the allowance from an existing non-zero allowance value, it will break a number of key functions or features of the protocol as the approve function is used inside key functions, such as FlashRolloverLoan_G5::_repayLoanFull for example:

function _repayLoanFull(
        uint256 _bidId,
        address _principalToken,
        uint256 _repayAmount
    ) internal returns (uint256 repayAmount_) {
        uint256 fundsBeforeRepayment = IERC20Upgradeable(_principalToken)
            .balanceOf(address(this));

        IERC20Upgradeable(_principalToken).approve(
            address(TELLER_V2),
            _repayAmount
        );
        TELLER_V2.repayLoanFull(_bidId);

        uint256 fundsAfterRepayment = IERC20Upgradeable(_principalToken)
            .balanceOf(address(this));
        repayAmount_ = fundsBeforeRepayment - fundsAfterRepayment;
    }

Code Snippet

The following attempt to call the approve() function without setting the allowance to zero first:

https://github.com/sherlock-audit/2024-04-teller-finance/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/FlashRolloverLoan_G5.sol#L194

IERC20Upgradeable(_flashToken).approve(
            address(POOL()),
            _flashAmount + _flashFees
        );

https://github.com/sherlock-audit/2024-04-teller-finance/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/FlashRolloverLoan_G5.sol#L243

IERC20Upgradeable(_principalToken).approve(
            address(TELLER_V2),
            _repayAmount
        );

Tool used

Manual Review

Recommendation

It is recommended to set the allowance to zero before increasing the allowance.

Duplicate of #140