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

Exploiting transform() to get free flash loan. #134

Closed
c4-bot-9 opened this issue Mar 13, 2024 · 4 comments
Closed

Exploiting transform() to get free flash loan. #134

c4-bot-9 opened this issue Mar 13, 2024 · 4 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 downgraded by judge Judge downgraded the risk level of this issue duplicate-435 🤖_44_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality

Comments

@c4-bot-9
Copy link
Contributor

Lines of code

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

Vulnerability details

Impact

The V3Vault.transform() allows users through a verified contract to transform a loan and check the health factor at the end of the function.

When V3Vault.transform() is called, transformedTokenId is set to tokenId of the loan to be transformed, and the function performs low-level calls in the data input.

  (bool success,) = transformer.call(data);
        if (!success) {
            revert TransformFailed();
        }

This call could be to any function in the vault except for ones where calls from the transformer were explicitly denied with a check(liquidate and decreaseLiquidityAndCollect ).

  function decreaseLiquidityAndCollect(DecreaseLiquidityAndCollectParams calldata params)
        external
        override
        returns (uint256 amount0, uint256 amount1)
    {
        // this method is not allowed during transform - can be called directly on nftmanager if needed from transform contract
        if (transformedTokenId > 0) {
            revert TransformNotAllowed();
        }
    function liquidate(LiquidateParams calldata params) external override returns (uint256 amount0, uint256 amount1) {
        // liquidation is not allowed during transformer mode
        if (transformedTokenId > 0) {
            revert TransformNotAllowed();
        }

The V3Vault.borrow(), calls are not rejected, and when borrowing in transform mode, loan health is not checked, this makes it possible to borrow undercollateralized loans in transform mode, since the check is only done at the end of the transform function.

Malicious users can exploit this to get free flash-loan. Couple with the fact that repay() does not have any restriction against transform mode, the loan can be repaid without any interest before control goes back to the transform function where loan health is checked.

Proof of Concept

A call through transform() to borrow() the largest possible amount considering available asset, GlobalDebtLimit, dailyDebtIncreaseLimitLeft and collateral threshold.

The user uses the asset in the same call and calls repay() with the borrowed amount to return the position to a healthy one in the same transaction.

function borrow(uint256 tokenId, uint256 assets) external override {
        bool isTransformMode =
            transformedTokenId > 0 && transformedTokenId == tokenId && transformerAllowList[msg.sender];

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

        _resetDailyDebtIncreaseLimit(newLendExchangeRateX96, false);
...
...
        // only does check health here if not in transform mode
        if (!isTransformMode) {
            _requireLoanIsHealthy(tokenId, debt);
        }
 function _repay(uint256 tokenId, uint256 amount, bool isShare, bytes memory permitData) internal { //@audit  repay in transform
        (uint256 newDebtExchangeRateX96, uint256 newLendExchangeRateX96) = _updateGlobalInterest();

Since all the transactions are happening in the same block, the loan is repaid with the same newDebtExchangeRateX96 that the loan was taken, so there is zero interest payment. All that is needed is for a user to have a loan position, even with the minimum acceptable amount.

    function _updateGlobalInterest()
        internal
        returns (uint256 newDebtExchangeRateX96, uint256 newLendExchangeRateX96)
    {
  @>>   // only needs to be updated once per block (when needed)
        if (block.timestamp > lastExchangeRateUpdate) {
            (newDebtExchangeRateX96, newLendExchangeRateX96) = _calculateGlobalInterest();
            lastDebtExchangeRateX96 = newDebtExchangeRateX96;
            lastLendExchangeRateX96 = newLendExchangeRateX96;
            lastExchangeRateUpdate = block.timestamp;
            emit ExchangeRateUpdate(newDebtExchangeRateX96, newLendExchangeRateX96);
        } else {
    @>>     newDebtExchangeRateX96 = lastDebtExchangeRateX96;
    @>>     newLendExchangeRateX96 = lastLendExchangeRateX96;
        }
    }

Tools Used

Manual review

Recommended Mitigation Steps

Check if in transform mode in repay() and charge a specific interest fee, since newDebtExchangeRateX96 cannot be updated twice in a single block.

Assessed type

Invalid Validation

@c4-bot-9 c4-bot-9 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Mar 13, 2024
c4-bot-10 added a commit that referenced this issue Mar 13, 2024
@c4-bot-11 c4-bot-11 added the 🤖_44_group AI based duplicate group recommendation label Mar 15, 2024
@c4-pre-sort
Copy link

0xEVom marked the issue as duplicate of #435

@c4-pre-sort c4-pre-sort added duplicate-435 sufficient quality report This report is of sufficient quality labels Mar 22, 2024
@c4-pre-sort
Copy link

0xEVom marked the issue as sufficient quality report

@c4-judge c4-judge added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value downgraded by judge Judge downgraded the risk level of this issue and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Mar 31, 2024
@c4-judge
Copy link

jhsagd76 changed the severity to 2 (Med Risk)

@c4-judge
Copy link

c4-judge commented Apr 1, 2024

jhsagd76 marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Apr 1, 2024
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 downgraded by judge Judge downgraded the risk level of this issue duplicate-435 🤖_44_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

4 participants