Revert Lend - VAD37's results

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L717-L725 https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L893-L898 https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L877-L917

Vulnerability details

In V3Vault.sol there all 3 instances of permit2.permitTransferFrom(), all 3 does not check token transfered in is USDC token. Allowing user to craft permit signature from any ERC20 token and Vault will accept it as USDC.

Impact

User can steal all USDC from vault using permit signature of any ERC20 token.

Proof of Concept

Here is how Vault accept USDC from user. Vault will accept Uniswap.Permit2 signature transfer allowance from Permit2 then to vault contract. https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L877C1-L917C6

    if (params.permitData.length > 0) {
        (ISignatureTransfer.PermitTransferFrom memory permit, bytes memory signature) =
            abi.decode(params.permitData, (ISignatureTransfer.PermitTransferFrom, bytes));
        permit2.permitTransferFrom(
            permit,
            ISignatureTransfer.SignatureTransferDetails(address(this), state.liquidatorCost),
            msg.sender,
            signature
        );
    } else {
        // take value from liquidator
        SafeERC20.safeTransferFrom(IERC20(asset), msg.sender, address(this), state.liquidatorCost);
    }

Below is permit signature struct that can be decoded from user provided data.

interface ISignatureTransfer is IEIP712 {
    /// @notice The token and amount details for a transfer signed in the permit transfer signature
    struct TokenPermissions {
        // ERC20 token address
        address token;
        // the maximum amount that can be spent
        uint256 amount;
    }

    /// @notice The signed permit message for a single token transfer
    struct PermitTransferFrom {
        TokenPermissions permitted;
        // a unique value for every token owner's signature to prevent signature replays
        uint256 nonce;
        // deadline on the permit signature
        uint256 deadline;
    }
}

V3Vault.sol need to check TokenPermissions.token is USDC, same as vault main asset.

Uniswap.permit2.permitTransferFrom() only check sign signature is correct. This is meaningless is Vault does not validate input data.

This allow user to use any ERC20 token. give allowance and permit to Uniswap.Permit2 Vault will accept any transfer token from Permit2 as USDC.

Allowing user to deposit any ERC20 token and steal USDC from vault.

Tools Used

Recommended Mitigation Steps

fix missing user input validation in 3 all instances of permit2 https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L717C1-L725C15 https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L893C1-L898C15 https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L877C1-L917C6

    if (params.permitData.length > 0) {
        (ISignatureTransfer.PermitTransferFrom memory permit, bytes memory signature) =
            abi.decode(params.permitData, (ISignatureTransfer.PermitTransferFrom, bytes));
        require(permit.permitted.token == asset, "V3Vault: invalid token");
        //@permitted amount is checked inside uniswap Permit2
        permit2.permitTransferFrom(
            permit,
            ISignatureTransfer.SignatureTransferDetails(address(this), state.liquidatorCost),
            msg.sender,
            signature
        );
    } else {
        // take value from liquidator
        SafeERC20.safeTransferFrom(IERC20(asset), msg.sender, address(this), state.liquidatorCost);
    }

Assessed type

ERC20

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L954-L988

Vulnerability details

V3Vault.repay() does not accept user input when user input more money repayment than debt. V3Vault.repay() not automatically clamp input down to reasonable value if user overshoot payment but revert.

This cause myriad of problems with V3Vault.Repay():

• Cannot calculate "perfect" USDC debt of user if transaction is pending in mainnet.
• Repay using more USDC than necessary as token input will always fail.
• Any periphery contract interact with vault.repay will have a hard time guess the correct amount of debt share to repay.
• repay() revert when user repay 99.9999% of debt. Which happen frequently because of exchange rate increment every block.
• If user only approve enough debt token of current block, repay with pending transaction will also fail
• user repay() transaction will revert if input share more than original debt.
• Anyone with freetime can frontrun repay() transaction by repaying tiny amount of debt. This prevent other user/contract from repaying "correct" debt.

The only correct way to repay debt is:

• approve USDC token to Vault more than debt amount.
• reading loan.debtShare from tokenId and call V3Vault.Repay() with these input (amount = debtShare, isShare = true)

This is suboptimal and repay function fail to work as intended if it does not accept (amount = 100% debt in token, isShare = false) as input. This cause problem with periphery contract that make call to V3Vault.Repay(). For example like LeverageTransformer.

Impact

V3Vault.repay() do not accept overpayment as input is quite major inconvenience for user and other contract. It introduce a lot of problems. Some of problem are damaging to user finance as it them more them to figure out how to repay 100% debt correctly.

Proof of Concept

Look at repay function. https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L964-L975

To simplify, I will call:

• repay with shareIn when repay with V3Vault.Repay(tokenId, amount, true,bytes(0)). amount here is loan debt calculated in Vault Share
• repay with assetIn when repay with V3Vault.Repay(tokenId, amount, false,bytes(0)). amount here is loan debt calculated in Vault Asset USDC
• User have 100$ USDC in debt and 80 share of debt need to repay for NFT.

List of problems

1. V3Vault.Repay() will simply revert if user input debt than necessary.

    function _repay(uint256 tokenId, uint256 amount, bool isShare, bytes memory permitData) internal {
        (uint256 newDebtExchangeRateX96, uint256 newLendExchangeRateX96) = _updateGlobalInterest();

        Loan storage loan = loans[tokenId];

        uint256 currentShares = loan.debtShares;

        uint256 shares;
        uint256 assets;

        if (isShare) {
            shares = amount;
            assets = _convertToAssets(amount, newDebtExchangeRateX96, Math.Rounding.Up);
        } else {
            assets = amount;
            shares = _convertToShares(amount, newDebtExchangeRateX96, Math.Rounding.Down);
        }

        // fails if too much repayed
        if (shares > currentShares) {
            revert RepayExceedsDebt();
        }

2. exchangeRate constantly updated and incremented every block. This make it impossible for user using normal UI, website to guess debt repayment amount correctly.

Look inside how global interest rate is updated.

(uint256 newDebtExchangeRateX96, uint256 newLendExchangeRateX96) = _updateGlobalInterest();

newDebtExchangeRateX96 and newLendExchangeRateX96 is incremented every block. That mean if user have 100$ USDC (debt + interest) using current DebtExchangeRateX96 to calculate. When user make transaction, repay 100$ USDC debt. Transaction is queued to next block. The debt already jump to 100.0001 $USDC debt. This causing converting, asset to shares always 99.9999% of debt share. shares = _convertToShares(amount, newDebtExchangeRateX96, Math.Rounding.Down);

3. Using wrong exchangeRate to calculate debt. User will approve slightly wrong amount of USDC to repay. If user have 100$ USDC, 80 share in debt this block, approving Vault to take 100$ USDC. When user make transaction, repay 80 share of debt. Transaction is queued to next block. The exchangeRate is accrued and increase 80 share of debt value already jump to 100.0001 $USDC. Now vault required to take 100.001$ USDC. Which is not enough allowance. This cause transaction to fail.

4. V3Vault.Repay() refuse to accept repayment if it is 99.999% of debt. https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L1006-L1012 Admin can set minimum loan size. This ussually 100 USDC or 10 USDC in test file.

Using wrong exchangeRate to calculate debt. User repay 100$ USDC now only worth 79.999 vault share. Because repay 99.9999% of debt is just a few cent missing. Repayment still fail and accrue more debt to user as more time go on.

5. User need to approve max USDC to vault and repay with shareIn 80 share. This broke repay function as it suppose to work with assetIn 100$ USDC as well.

6. Periphery contract or bot contract like LeverageTransformer cannot make call to V3Vault.Repay() correctly. https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/transformers/LeverageTransformer.sol#L165-L166

LeverageTransformer might collect more interest than debt and repay more USDC than necessary. This transaction will fail due to repay too much token.

7. Random user can frontrun repayment. Cancel other user repayment by repay tiny amount of debt first. When user try to repay 80 share of debt. But loan now only require to repay 79.99 share of debt due to someone front run them first. This will also revert due to repay too much token.

Tools Used

manual

Recommended Mitigation Steps

V3Vault.Repay() should accept overpayment as input. This is the only correct way to repay debt.

    function _repay(uint256 tokenId, uint256 amount, bool isShare, bytes memory permitData) internal {
        (uint256 newDebtExchangeRateX96, uint256 newLendExchangeRateX96) = _updateGlobalInterest();

        Loan storage loan = loans[tokenId];

        uint256 currentShares = loan.debtShares;

        uint256 shares;
        uint256 assets;

        if (isShare) {
            if(amount > currentShares) 
                shares = currentShares;            
            else
                shares = amount;
            assets = _convertToAssets(amount, newDebtExchangeRateX96, Math.Rounding.Up);
        } else {
            uint maxAssets = _convertToAssets(currentShares, newDebtExchangeRateX96, Math.Rounding.Up);
            if(amount > maxAssets) 
                assets = maxAssets;            
            else assets = amount;            
            shares = _convertToShares(amount, newDebtExchangeRateX96, Math.Rounding.Down);
        }

Assessed type

Other