Flashloan rollover doesn't work with USDT

Summary

Teller finance introduces a feature that allows for loan rollover using a flash loan. In one transaction the old loan is repaid and a new one is created. The issue is that not all popular tokens on Ethereum are fully ERC20 compliant. Thus, the rollover feature will not work with these tokens.

One such example is the USDT token. As mentioned in the contest readme, the protocol should be able to work with it:

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 .

Vulnerability Detail

USDT is a major token, supported by Uniswap V3 and according to the contest rules should be supported by the protocol. Unfortunately, said token has some weird behaviour like - approval race protection and missing return values - making integrations with is more intricate.

Impact

Flashloan rollover feature is unusable with not fully ERC20 compliant tokens. This renders key protocol functionality unusable with one of the biggest tokens in crypto.

Code Snippet

The FlashRolloverLoan_G5 contract uses the standard transferFrom, approve and transfer functions, which are problematic when working with USDT.

The following PoC demonstrates how flashloan rollover fails when used on mainnet with USDT.

You can swap tokenAddress value to see that the same test will succeed with USDC

Add rpc url to .env file MAINNET_RPC_URL="https://…”

Set rpc config in foundry.toml:

[rpc_endpoints] 
mainnet = "${MAINNET_RPC_URL}"

Add the following code under contracts/tests/LenderCommitmentForwarder/extensions/USDTAppoveFlashLoan.t.sol
```
pragma solidity ^0.8.0;
```

import { Testable } from "../../../Testable.sol"; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

import { FlashRolloverLoan } from "../../../../contracts/LenderCommitmentForwarder/extensions/FlashRolloverLoan.sol";

import "../../../../contracts/interfaces/ILenderCommitmentForwarder.sol"; import "../../../../contracts/interfaces/IFlashRolloverLoan_G4.sol";

import "../../../integration/IntegrationTestHelpers.sol";

import { TellerV2SolMock } from "../../../../contracts/mock/TellerV2SolMock.sol"; import { LenderCommitmentForwarderMock } from "../../../../contracts/mock/LenderCommitmentForwarderMock.sol"; import { MarketRegistryMock } from "../../../../contracts/mock/MarketRegistryMock.sol";

import { AavePoolAddressProviderMock } from "../../../../contracts/mock/aave/AavePoolAddressProviderMock.sol"; import { AavePoolMock } from "../../../../contracts/mock/aave/AavePoolMock.sol";

import { FlashRolloverLoan_G5 } from "../../../../contracts/LenderCommitmentForwarder/extensions/FlashRolloverLoan_G5.sol";

contract FlashRolloverLoanOverride is FlashRolloverLoan_G5 { constructor( address _tellerV2, address _lenderCommitmentForwarder, address _aaveAddressProvider ) FlashRolloverLoan_G5( _tellerV2, _aaveAddressProvider ) {}

function acceptCommitment(
    address _lenderCommitmentForwarder,
    address borrower,
    address principalToken,
    FlashRolloverLoan_G5.AcceptCommitmentArgs calldata _commitmentArgs
) public returns (uint256 bidId_, uint256 acceptCommitmentAmount_) {
    return
        super._acceptCommitment(_lenderCommitmentForwarder,borrower, principalToken, _commitmentArgs);
}

}

contract USDTFlashLoanRollover is Testable { constructor() {}

address private borrower;
address private lender;

AavePoolMock aavePoolMock;
AavePoolAddressProviderMock aavePoolAddressProvider;
FlashRolloverLoanOverride flashRolloverLoan;
TellerV2SolMock tellerV2;
IERC20 usdt;
LenderCommitmentForwarderMock lenderCommitmentForwarder;
MarketRegistryMock marketRegistryMock;

address smartCommitmentForwarderAddress;

function setUp() public {
    vm.createSelectFork(vm.rpcUrl("mainnet"));
    vm.roll(1);
    vm.warp(1);

    borrower = makeAddr("borrower");
    lender = makeAddr("lender");

    tellerV2 = new TellerV2SolMock();
    address tokenAddress = 0xdAC17F958D2ee523a2206206994597C13D831ec7; // USDT
    // address tokenAddress = 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48; // USDC - running the test with ERC20 compliant token succeeds
    vm.label(tokenAddress, "USDT");

    usdt = IERC20(address(tokenAddress));

    marketRegistryMock = new MarketRegistryMock();

    tellerV2.setMarketRegistry(address(marketRegistryMock));

    lenderCommitmentForwarder = new LenderCommitmentForwarderMock();

    aavePoolAddressProvider = new AavePoolAddressProviderMock(
        "marketId",
        address(this)
    );

    aavePoolMock = new AavePoolMock();
    vm.label(address(aavePoolMock), "AavePoolMock");

    bytes32 POOL = "POOL";
    aavePoolAddressProvider.setAddress(POOL, address(aavePoolMock));

    deal(address(usdt), lender, 5e18);
    deal(address(usdt), borrower, 5e18);
    deal(address(usdt), address(lenderCommitmentForwarder), 5e18);
    deal(address(usdt), address(aavePoolMock), 5e18);

    flashRolloverLoan = new FlashRolloverLoanOverride(
        address(tellerV2),
        address(lenderCommitmentForwarder),
        address(aavePoolAddressProvider)
    );

    IntegrationTestHelpers.deployIntegrationSuite();
}

function test_RolloverUSDTLoanWithFlash() public {
    address lendingToken = address(usdt);
    uint256 marketId = 0;
    uint256 principalAmount = 500;
    uint32 duration = 10 days;
    uint16 interestRate = 100;

    ILenderCommitmentForwarder.Commitment
        memory commitment = ILenderCommitmentForwarder.Commitment({
            maxPrincipal: principalAmount,
            expiration: uint32(block.timestamp + 1 days),
            maxDuration: duration,
            minInterestRate: interestRate,
            collateralTokenAddress: address(0),
            collateralTokenId: 0,
            maxPrincipalPerCollateralAmount: 0,
            collateralTokenType: ILenderCommitmentForwarder
                .CommitmentCollateralType
                .NONE,
            lender: lender,
            marketId: marketId,
            principalTokenAddress: lendingToken
        });

    lenderCommitmentForwarder.setCommitment(0, commitment);

    FlashRolloverLoan_G5.AcceptCommitmentArgs
        memory commitmentArgs = FlashRolloverLoan_G5.AcceptCommitmentArgs({
            commitmentId: 0,
            smartCommitmentAddress: smartCommitmentForwarderAddress,
            principalAmount: principalAmount,
            collateralAmount: 100,
            collateralTokenId: 0,
            collateralTokenAddress: address(0),
            interestRate: interestRate,
            loanDuration: duration,
            merkleProof: new bytes32[](0)
        });

    vm.startPrank(borrower);
    uint256 loanId = tellerV2.submitBid(
        lendingToken,
        marketId,
        principalAmount,
        duration,
        interestRate,
        "",
        borrower
    );

    uint256 flashAmount = 500;
    uint256 borrowerAmount = 5; // aave fee

    SafeERC20.safeApprove(IERC20(lendingToken), address(flashRolloverLoan), 1e18);

    flashRolloverLoan.rolloverLoanWithFlash(
        address(lenderCommitmentForwarder),
        loanId,
        flashAmount,
        borrowerAmount,
        commitmentArgs
    );

    bool flashLoanSimpleWasCalled = aavePoolMock.flashLoanSimpleWasCalled();
    assertTrue(
        flashLoanSimpleWasCalled,
        "flashLoanSimpleWasCalled not called"
    );
}

}



## Tool used

Manual Review

## Recommendation

Use the `SafeERC20` library in `FlashRolloverLoan_G5`. Update the necessary mocks so tests could run as well. Here is a [patch](https://gist.github.com/georgiIvanov/0346be833b4743d52b0f0832fc6e8155) with all the files that need an update.

Duplicate of #140

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

givn - Flashloan rollover doesn't work with USDT #266

Flashloan rollover doesn't work with USDT

Summary

Vulnerability Detail

Impact

Code Snippet