Venus Prime - Flora's results

Lines of code

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

Vulnerability details

Impact

If a user who would be issued an irrevocable token has staked more than 1,000 XVS tokens for at least 90 days. After their irrevocable token gets burnt, the vulnerability allows the user to immediately become eligible for claiming a revocable token as long as they have more than 1,000 staked XVS, without having to wait for the minimum required waiting period.

Here is the scenario

Suppose a user, who meets the criteria to receive an irrevocable token, has staked more than 1,000 XVS tokens for at least 90 days - they are eligible to claim revocable token, but have yet to claim one. At this point in time, their staked time has been recorded in the Prime.sol contract (block.timestamp - stakedAt[user] >= STAKING_PERIOD)

Scenario steps:

1. The protocol issues irrevocable to that user.
2. The user unstakes their XVS tokens, reducing their staked balance to less than 1,000 XVS.
3. The user’s irrevocable token gets burnt by the governance for some reason afterward (it can be because they no longer stake any XVS tokens, as stated by the sponsor: “In which case the user's prime token would be burnt by the external burn fn? when the Venus community decides it. There could be several criteria. I can imagine some of them (they are not defined yet), for example, the Venus community could decide to burn an irrevocable token if the user unstaked almost 100% of their XVS and they stopped using the protocol for a long time (this is only an example, provided by me, not by the Venus community)”).
4. The user restakes more than 1,000 XVS XVS on the platform. At this point, they should not be eligible to claim a revocable token because they have yet to meet the waiting period requirement. However, because their staked time (stakedAt[user]) has not been reset when they were issued the irrevocable token, they’ll immediately become eligible to claim revocable token without having to wait.

Proof of Concept

Here is the coded scenario to illustrate the vulnerability:

  describe("mint and burn", () => {
    let prime: PrimeScenario;
    let xvsVault: XVSVault;
    let xvs: XVS;

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

    it.only("POC - user immediately gets eligible for claiming revocable token without having to wait for the minimum period", async () => {
      // user stakes 10000 XVS for the last 90 days
      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);

      // user gets issued irrevocable token
      await prime.issue(true, [user1.getAddress()]);

      // user unstakes XVS
      await xvsVault.connect(user1).requestWithdrawal(xvs.address, 0, bigNumber18.mul(10000));

      // user’s irrevocable token gets burned by the governance for some reason (can be because they no longer stake any XVS tokens)
      await prime.burn(user1.getAddress());
      let irrevocableToken = await prime.tokens(user1.getAddress());
      expect(irrevocableToken.isIrrevocable).to.be.equal(false);
      expect(irrevocableToken.exists).to.be.equal(false);

      // user restakes >1k XVS to the platform
      await xvs.connect(user1).approve(xvsVault.address, bigNumber18.mul(10000));
      await xvsVault.connect(user1).deposit(xvs.address, 0, bigNumber18.mul(10000));

      // they’ll immediately get eligible for claiming revocable token without having to wait for the minimum period.
      await expect(prime.connect(user1).claim()).to.be.not.reverted;
      let revocableToken = await prime.tokens(user1.getAddress());
      expect(revocableToken.isIrrevocable).to.be.equal(false);
      expect(revocableToken.exists).to.be.equal(true);
    });
  });

Tools Used

VsCode

Recommended Mitigation Steps

Reset the staked time to 0 after issuing irrevocable token for users

    /**
     * @notice Directly issue prime tokens to users
     * @param isIrrevocable are the tokens being issued
     * @param users list of address to issue tokens to
     */
    function issue(bool isIrrevocable, address[] calldata users) external {
        _checkAccessAllowed("issue(bool,address[])");

        if (isIrrevocable) {
            for (uint256 i = 0; i < users.length; ) {
                Token storage userToken = tokens[users[i]];
                if (userToken.exists && !userToken.isIrrevocable) {
                    _upgrade(users[i]);
                } else {
                    _mint(true, users[i]);
                    _initializeMarkets(users[i]);
                    stakedAt[users[i]] = 0;
                }

                unchecked {
                    i++;
                }
            }
        } else {
            for (uint256 i = 0; i < users.length; ) {
                _mint(false, users[i]);
                _initializeMarkets(users[i]);
                delete stakedAt[users[i]];

                unchecked {
                    i++;
                }
            }
        }
    }

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

Stated in the discord channel:

“What is the primary use of the issue function?

Issue the initial set of Prime tokens, to the users that were staking more than 1,000 XVS tokens for the last 90 days. Moreover, it will be used to issue the irrevocable tokens (the business criteria for issuing these irrevocable tokens are not defined yet)”.

A user can bypass these criteria by first becoming eligible to be issued a revocable token, then frontrunning the issue function and unstaking their XVS tokens (resulting in their staked XVS balance dropping below 1,000 XVS).

Since the issue function lacks checks when minting a revocable token, the user would still receive a revocable token when the issue function is executed, even though they no longer meet the qualification criteria (they no longer stake >= 1000 XVS tokens).

Proof of Concept

Here is the coded scenario to illustrate the vulnerability

  describe("mint and burn", () => {
    let prime: PrimeScenario;
    let xvsVault: XVSVault;
    let xvs: XVS;

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

    it.only("POC - user still receives a revocable token when the `issue` function is executed, even though they’re no longer qualified.", async () => {
      // user stakes 10000 XVS for the last 90 days - make them eligible for being issued a revocable token by the protocol
      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);

      // user frontruns `issue` function to unstake XVS tokens
      await xvsVault.connect(user1).requestWithdrawal(xvs.address, 0, bigNumber18.mul(9700));

      // user gets issued revocable token, despite not staking more than 1000 XVS tokens
      await expect(prime.issue(false, [user1.getAddress()])).to.be.not.reverted;
      let token = await prime.tokens(user1.getAddress());
      expect(token.isIrrevocable).to.be.equal(false);
      expect(token.exists).to.be.equal(true);
    });
  });

Tools Used

VsCode

Recommended Mitigation Steps

In issue function, add a requirement to check if a user has staked a sufficient amount of XVS tokens before minting to them a revocable token

    /**
     * @notice Directly issue prime tokens to users
     * @param isIrrevocable are the tokens being issued
     * @param users list of address to issue tokens to
     */
    function issue(bool isIrrevocable, address[] calldata users) external {
        _checkAccessAllowed("issue(bool,address[])");

        if (isIrrevocable) {
            for (uint256 i = 0; i < users.length; ) {
                Token storage userToken = tokens[users[i]];
                if (userToken.exists && !userToken.isIrrevocable) {
                    _upgrade(users[i]);
                } else {
                    _mint(true, users[i]);
                    _initializeMarkets(users[i]);
                }

                unchecked {
                    i++;
                }
            }
        } else {
            for (uint256 i = 0; i < users.length; ) {
				uint256 totalStaked = _xvsBalanceOfUser(users[i]);
                bool isAccountEligible = isEligible(totalStaked);
                if (!isAccountEligible) {
                    revert IneligibleToClaim();
                }
                _mint(false, users[i]);
                _initializeMarkets(users[i]);
                delete stakedAt[users[i]];

                unchecked {
                    i++;
                }
            }
        }
    }

Assessed type

MEV