search

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

6 stars 6 forks source link

pkqs90 - `FlashRolloverLoan_G5#rolloverLoanWithFlash` does not support fee-on-transfer tokens #275

Closed sherlock-admin2 closed 2 months ago

sherlock-admin2 commented 2 months ago

pkqs90

medium

FlashRolloverLoan_G5#rolloverLoanWithFlash does not support fee-on-transfer tokens

Summary

When borrower calls rolloverLoanWithFlash(), for fee-on-transfer tokens, the contract may pay back the borrower more tokens than expected, causing a loss of funds, or even the entire call will revert due to not enough tokens.

Vulnerability Detail

In FlashRolloverLoan_G5#rolloverLoanWithFlash, users pass in a _borrowerAmount defining the amount of tokens that is used for this rolloverLoan action. However, for fee-on-transfer tokens, the actual amount of tokens is less than _borrowerAmount.

When the flashRollover action is finished, the remaining tokens are transferred back to the borrower, however, it assumes the contract received _rolloverArgs.borrowerAmount during the initial transfer, which is not true. This will end up paying back the borrower more tokens than expected, causing a loss of funds, or even the entire call will revert due to not enough tokens.

    function rolloverLoanWithFlash(
        address _lenderCommitmentForwarder,
        uint256 _loanId,
        uint256 _flashLoanAmount,
        uint256 _borrowerAmount, //an additional amount borrower may have to add
        AcceptCommitmentArgs calldata _acceptCommitmentArgs
    ) external   {
        address borrower = TELLER_V2.getLoanBorrower(_loanId);
        require(borrower == msg.sender, "CommitmentRolloverLoan: not borrower");

        // Get lending token and balance before
        address lendingToken = TELLER_V2.getLoanLendingToken(_loanId);

        if (_borrowerAmount > 0) {
            IERC20(lendingToken).transferFrom(
                borrower,
                address(this),
>               _borrowerAmount
            );
        }
    }

    function executeOperation(
        address _flashToken,
        uint256 _flashAmount,
        uint256 _flashFees,
        address _initiator,
        bytes calldata _data
    ) external virtual onlyFlashLoanPool returns (bool) {
        ...
        uint256 fundsRemaining = acceptCommitmentAmount +
>           _rolloverArgs.borrowerAmount -
            repaymentAmount -
            _flashFees;

        if (fundsRemaining > 0) {
            IERC20Upgradeable(_flashToken).transfer(
                _rolloverArgs.borrower,
                fundsRemaining
            );
        }
    }

Note that the contest README states that any tokens compatible with Uniswap V3 should be supported, which includes fee-on-transfer tokens.

We are allowing any standard token that would be compatible with Uniswap V3 to work with our codebase, just as was the case for the original audit of TellerV2.sol. The tokens are assumed to be able to work with Uniswap V3.

Impact

For fee-on-transfer tokens, the contract may pay back the borrower more tokens than expected, causing a loss of funds, or even the entire call will revert due to not enough tokens.

Code Snippet

Tool used

Manual review

Recommendation

Use the diff of tokens after calling IERC20(lendingToken).transferFrom instead of the user passed-in _flashLoanAmount.

Duplicate of #122