Venus Prime - 0xpiken's results

Lines of code

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L335-L346

Vulnerability details

Impact

User has a chance to claim a revocable prime token after their irrevocable token is burnt, even the amount their staked XVS token is less than 1,000. Hereafter they can claim interest reward even they are not eligible. The rules of claiming Venus Prime will be broken and other eligible users could suffer losses on their interest reward.

Proof of Concept

The AccessControlManager who has permissions can call issue() to mint irrevocable prime token to any user no matter how many XVS tokens they staked.

335:            for (uint256 i = 0; i < users.length; ) {
336:                Token storage userToken = tokens[users[i]];
337:                if (userToken.exists && !userToken.isIrrevocable) {
338:                    _upgrade(users[i]);
339:                } else {
340:                    _mint(true, users[i]);
341:                    _initializeMarkets(users[i]);
342:                }
343:
344:                unchecked {
345:                    i++;
346:                }
347:            }

Only the AccessControlManager who has permissions can call burn() to revoke the irrevocable prime token.

411:    function burn(address user) external {
412:        _checkAccessAllowed("burn(address)");
413:        _burn(user);
414:    }

When the irrevocable prime token is burnt, user should claim no interest reward hereafter. If they want to claim interest reward again, first they need to meet the requirements of staking XVS token (staking no less than 1,000 XVS and at least 90 days) and then mint a revocable prime token.

But when an irrevocable prime token is minted, startAt is not reset to 0, the owner of this token could intentionally or unintentionally mint a revocable prime token immediately after their irrevocable prime token is burnt even they don't have enough XVS token staked. e.g.:

1. User 1 stakes 10,000 XVS token
2. 90 days later, AccessControlManager issues an irrevocable token to user 1
3. User 1 withdraws 9,500 XVS token
4. AccessControlManager burns the irrevocable prime token of user 1
5. User 1 claims a revocable prime token even the amount their staked XVS token is only 500, and continue to claim interest reward as long as no one call xvsUpdated(user1) to revoke their revocable prime token.

Copy the below code into Prime.ts and run it:

  describe("Issue irrevocable prime token", () => {
    let prime: PrimeScenario;
    let xvsVault: XVSVault;
    let xvs: XVS;

    beforeEach(async () => {
      ({
        prime,
        xvsVault,
        xvs,
      } = await loadFixture(deployProtocol));

      await xvs.connect(user1).approve(xvsVault.address, bigNumber18.mul(10000));
      await xvsVault.connect(user1).deposit(xvs.address, 0, bigNumber18.mul(10000));
      await mine(90 * 24 * 60 * 60);
      await prime.issue(true, [user1.getAddress()]);
    });
    it("claim revocable prime token after the irrevocable prim token is revoked/burnt", async () => {
      //@audit-info The irrevocable prime token has been minted
      let token = await prime.tokens(user1.getAddress());
      expect(token.exists).to.be.equal(true);
      expect(token.isIrrevocable).to.be.equal(true);
      //@audit-info stakeAt is not cleared, which let user1 have chance to claim revocable prime token immediatelly after the irrevocable prime token is revoked/burnt. 
      let stakeAt = await prime.stakedAt(user1.getAddress());
      expect(stakeAt).be.gt(0);
      //@audit-info user1 withdraw 9500 XVS token, only 500 left in xvsVault
      await xvsVault.connect(user1).requestWithdrawal(xvs.address, 0, bigNumber18.mul(9500));
      let [xvsAmount, rewardDebt , pendingWithdrawals] = await xvsVault.getUserInfo(xvs.address, 0, user1.getAddress());
      expect(xvsAmount.sub(pendingWithdrawals)).be.equal(bigNumber18.mul(500));
      //@audit-info revoke the irrevocable prime token of user1
      await prime.burn(user1.getAddress());
      //@audit-info The irrevocable prime token has been revoked sucessfully
      token = await prime.tokens(user1.getAddress());
      expect(token.exists).to.be.equal(false);
      expect(token.isIrrevocable).to.be.equal(false);
      //@audit-info user1 claim revocable prime token immediatelly
      await prime.connect(user1).claim();
      //@audit-info The revocable prime token has been minted
      token = await prime.tokens(user1.getAddress());
      expect(token.exists).to.be.equal(true);
      expect(token.isIrrevocable).to.be.equal(false);
    });
  });

Tools Used

Manual review

Recommended Mitigation Steps

Reset startAt to 0 when minting an irrevocable prime token in function issue():

335:            for (uint256 i = 0; i < users.length; ) {
336:                Token storage userToken = tokens[users[i]];
337:                if (userToken.exists && !userToken.isIrrevocable) {
338:                    _upgrade(users[i]);
339:                } else {
340:                    _mint(true, users[i]);
341:                    _initializeMarkets(users[i]);
+                       delete stakedAt[users[i]];
342:                }
343:
344:                unchecked {
345:                    i++;
346:                }
347:            }

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L872-L897

Vulnerability details

Impact

User may exploit extra profit or suffer loss on interest rewards because of the flaw of Prime#_capitalForScore()

Proof of Concept

Any eligible user can claim a revocable prime token or be issued an irrevocable/revocable prime token. The user who owns a prime token can claim interest reward through claimInterest(). The accrued interest reward is production of the score of a user in a market and the delta of reward index: https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L922

        return (index * score) / EXP_SCALE;

the score is calculated on staked xvs token value and amount of borrow / supply in the market: https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L647-L664

    function _calculateScore(address market, address user) internal returns (uint256) {
        uint256 xvsBalanceForScore = _xvsBalanceForScore(_xvsBalanceOfUser(user));

        IVToken vToken = IVToken(market);
        uint256 borrow = vToken.borrowBalanceStored(user);
        uint256 exchangeRate = vToken.exchangeRateStored();
        uint256 balanceOfAccount = vToken.balanceOf(user);
        uint256 supply = (exchangeRate * balanceOfAccount) / EXP_SCALE;

        address xvsToken = IXVSVault(xvsVault).xvsAddress();
        oracle.updateAssetPrice(xvsToken);
        oracle.updatePrice(market);

        (uint256 capital, , ) = _capitalForScore(xvsBalanceForScore, borrow, supply, market);
        capital = capital * (10 ** (18 - vToken.decimals()));

        return Scores.calculateScore(xvsBalanceForScore, capital, alphaNumerator, alphaDenominator);
    }

Each market has a marketBorrowMultipler and marketSupplyMultipler, which are used to calculate the borrow cap or supply cap in USD. If the amount of borrow / supply in USD exceeds the cap, the amount of borrow / supply will be recalculated: https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L872-L897

    function _capitalForScore(
        uint256 xvs,
        uint256 borrow,
        uint256 supply,
        address market
    ) internal view returns (uint256, uint256, uint256) {
        address xvsToken = IXVSVault(xvsVault).xvsAddress();

        uint256 xvsPrice = oracle.getPrice(xvsToken);
        uint256 borrowCapUSD = (xvsPrice * ((xvs * markets[market].borrowMultiplier) / EXP_SCALE)) / EXP_SCALE;
        uint256 supplyCapUSD = (xvsPrice * ((xvs * markets[market].supplyMultiplier) / EXP_SCALE)) / EXP_SCALE;

        uint256 tokenPrice = oracle.getUnderlyingPrice(market);
        uint256 supplyUSD = (tokenPrice * supply) / EXP_SCALE;
        uint256 borrowUSD = (tokenPrice * borrow) / EXP_SCALE;

        if (supplyUSD >= supplyCapUSD) {
            supply = supplyUSD > 0 ? (supply * supplyCapUSD) / supplyUSD : 0;
        }

        if (borrowUSD >= borrowCapUSD) {
            borrow = borrowUSD > 0 ? (borrow * borrowCapUSD) / borrowUSD : 0;
        }

        return ((supply + borrow), supply, borrow);
    }

Suppose that two users stake same amount of xvs and supply/borrow same amount of underlying token, and both of them reached capped supply/borrow. Since the prices of xvs and the underlying token could be changed over time, they may get different scores because of token price change and claim different interest rewards.

Some users may call accrueInterestAndUpdateScore() to maximize their score to get extra interest rewards and the others will suffer losses of interest rewards because of this flaw.

When cap is not reached, different users staking same amount of xvs and supplying same amount of underlying token have same scores: user1 stakes 2000 xvs user1 supplies 1 bnb the price of xvs is 3 USD the price of bnb is 300 USD since the cap is not reached, the score of user1 in bnb market is 2000^0.5 * 1^(1-0.5) = 44.721359549995802910 user2 stakes 2000 xvs user2 supplies 1 bnb the price of xvs is 6 USD the price of bnb is 300 USD since the cap is not reached, the score of user2 in bnb market is 2000^0.5 * 1^(1-0.5) = 44.721359549995802910 the score of user1 in bnb market is same as user2.

• When cap is not reached, different user staking same amount of xvs and supplying smae amount of underlying token have same scores:

  • user1:
    • stakes 2000 xvs
    • supplies 1 bnb
    • the price of xvs is 3 USD
    • the price of bnb is 300 USD
    • since the cap is not reached, the score of user1 in bnb market is 2000^0.5 * 1^(1-0.5) = 44.721359549995802910
  • user2:
    • stakes 2000 xvs
    • supplies 1 bnb
    • the price of xvs is 6 USD
    • the price of bnb is 300 USD
    • since the cap is not reached, the score of user2 in bnb market is 2000^0.5 * 1^(1-0.5) = 44.721359549995802910
  • the score of user2 in bnb market is same as user1.

• When cap is reached, different user staking same amount of xvs and supplying smae amount of underlying token may have different scores:

  • user1:
    • stakes 2000 xvs
    • supplies 50 bnb
    • the price of xvs is 3 USD
    • the price of bnb is 300 USD
    • since the cap is reached, the score of user1 in bnb market is 2000^0.5 * 20^(1-0.5) = 200.000000000000029058
  • user2:
    • stakes 2000 xvs
    • user2 supplies 1 bnb
    • the price of xvs is 6 USD at this moment
    • the price of bnb is 300 USD
    • since the cap is not reached, the score of user2 in bnb market is 2000^0.5 * 40^(1-0.5) = 282.842712474619027442
  • the score of user2 in bnb market is different with user1

Copy blow codes to Prime.ts and run it:

  describe.only("update xvs price", () => {
    let comptroller: MockContract<ComptrollerMock>;
    let prime: PrimeScenario;
    let vusdt: VBep20Harness;
    let veth: VBep20Harness;
    let xvsVault: XVSVault;
    let xvs: XVS;
    let oracle: FakeContract<ResilientOracleInterface>;
    let primeLiquidityProvider: PrimeLiquidityProvider;

    let vbnb: VBep20Harness;
    let bnb: BEP20Harness;
    let xvsPriceUpdated: boolean;
    beforeEach(async () => {
      ({
        comptroller,
        prime,
        vusdt,
        veth,
        xvsVault,
        xvs,
        oracle,
        primeLiquidityProvider,
      } = await loadFixture(deployProtocol));
      const tokenFactory = await ethers.getContractFactory("BEP20Harness");
      bnb = (await tokenFactory.deploy(
        bigNumber18.mul(100000000),
        "bnb",
        BigNumber.from(18),
        "BEP20 bnb",
      )) as BEP20Harness;

      const interestRateModelHarnessFactory = await ethers.getContractFactory("InterestRateModelHarness");
      const InterestRateModelHarness = (await interestRateModelHarnessFactory.deploy(
        BigNumber.from(18).mul(5),
      )) as InterestRateModelHarness;

      const vTokenFactory = await ethers.getContractFactory("VBep20Harness");
      vbnb = (await vTokenFactory.deploy(
        bnb.address,
        comptroller.address,
        InterestRateModelHarness.address,
        bigNumber18,
        "VToken bnb",
        "vbnb",
        BigNumber.from(18),
        deployer.getAddress(),
      )) as VBep20Harness;

      await vbnb._setReserveFactor(bigNumber16.mul(20));
      await primeLiquidityProvider.initializeTokens([bnb.address]);

      oracle.getUnderlyingPrice.returns((vToken: string) => {
        if (vToken == vusdt.address) {
          return convertToUnit(1, 18);
        } else if (vToken == veth.address) {
          return convertToUnit(1200, 18);
        } else if (vToken == vbnb.address) {
          return convertToUnit(300, 18);
        }
      });

      oracle.getPrice.returns((token: string) => {
        if (token == xvs.address) {
          if (xvsPriceUpdated) {
            return convertToUnit(6, 18);
          } else {
            return convertToUnit(3, 18);
          }
        }
      });

      const half = convertToUnit("0.5", 8);
      await comptroller._supportMarket(vbnb.address);
      await comptroller._setCollateralFactor(vbnb.address, half);

      bnb.transfer(user1.getAddress(), bigNumber18.mul(100));
      bnb.transfer(user2.getAddress(), bigNumber18.mul(100));

      await comptroller._setMarketSupplyCaps([vbnb.address], [bigNumber18.mul(100)]);
      await comptroller._setMarketBorrowCaps([vbnb.address], [bigNumber18.mul(100)]);

      await comptroller._setPrimeToken(prime.address);
    });

    it("stake same amount of xvs, supply same amount of bnb, calculate the score with differnt xvs price", async () => {
      xvsPriceUpdated = false;
      await bnb.connect(user1).approve(vbnb.address, bigNumber18.mul(1));
      await vbnb.connect(user1).mint(bigNumber18.mul(1));
      await bnb.connect(user2).approve(vbnb.address, bigNumber18.mul(1));
      await vbnb.connect(user2).mint(bigNumber18.mul(1));

      await prime.addMarket(vbnb.address, bigNumber18.mul(1), bigNumber18.mul(1));

      await xvs.connect(user1).approve(xvsVault.address, bigNumber18.mul(2000));
      await xvsVault.connect(user1).deposit(xvs.address, 0, bigNumber18.mul(2000));
      await xvs.connect(user2).approve(xvsVault.address, bigNumber18.mul(2000));
      await xvsVault.connect(user2).deposit(xvs.address, 0, bigNumber18.mul(2000));
      await prime.issue(false, [user1.getAddress(), user2.getAddress()]);

      let interest = await prime.interests(vbnb.address, user1.getAddress());
      /*
       * the score of user 1 when the price of xvs is 3 USD: 2000^0.5 * 1^(1-0.5)
      */
      expect(interest.score.toString()).to.be.equal("44721359549995802910");
      
      xvsPriceUpdated = true;
      await prime.accrueInterestAndUpdateScore(user2.getAddress(), vbnb.address);
      interest = await prime.interests(vbnb.address, user2.getAddress());
      /*
       * the score of user 2 when the price of xvs is 6 USD: 2000^0.5 * 1^(1-0.5)
      */
      expect(interest.score.toString()).to.be.equal("44721359549995802910");

      xvsPriceUpdated = false;
      await bnb.connect(user1).approve(vbnb.address, bigNumber18.mul(49));
      await vbnb.connect(user1).mint(bigNumber18.mul(49));
      await bnb.connect(user2).approve(vbnb.address, bigNumber18.mul(49));
      await vbnb.connect(user2).mint(bigNumber18.mul(49));
      interest = await prime.interests(vbnb.address, user1.getAddress());
      /*
       * the score of user 1 when the price of xvs is 3 USD(supply cap reached): 2000^0.5 * 20^(1-0.5)
      */
      expect(interest.score.toString()).to.be.equal("200000000000000029058");

      xvsPriceUpdated = true;
      await prime.accrueInterestAndUpdateScore(user2.getAddress(), vbnb.address);
      interest = await prime.interests(vbnb.address, user2.getAddress());
      /*
       * the score of user 2 when the price of xvs is 6 USD(supply cap reached): 2000^0.5 * 40^(1-0.5)
      */
      expect(interest.score.toString()).to.be.equal("282842712474619027442");
    });
  });

It's easy to notice that when supply cap is reached, the scores of user1 and user2 are different simply because the prices of xvs are different when these scores are calculated.

Tools Used

Manual review

Recommended Mitigation Steps

Consider defining cappedSupplyAmount and cappedBorrowAmount, the number limitations of underlying token, for each market to calculate the score. E.g. change the code of _capitalForScore() as below:

    function _capitalForScore(
        uint256 xvs,
        uint256 borrow,
        uint256 supply,
        address market
    ) internal view returns (uint256, uint256, uint256) {
        if (supplyUSD > markets[market].cappedSupplyAmount) {
            supplyUSD = markets[market].cappedSupplyAmount;
        }
        if (borrowUSD > markets[market].cappedBorrowAmount) {
            borrowUSD = markets[market].cappedBorrowAmount;
        }
        return ((supply + borrow), supply, borrow);
    }

Assessed type

Other