Venus Prime - imare's results

Lines of code

https://github.com/code-423n4/2023-09-venus/blob/b11d9ef9db8237678567e66759003138f2368d23/contracts/Tokens/Prime/Prime.sol#L725-L756

Vulnerability details

After a token burn happens even if the user has enough staked xvs and has again waited the waiting period will not get a new token back.

The burn operation will not restart the waiting period as it should. The user must do a new deposit to the XVSVault where Prime#xvsUpdated is automatically called or call the Prime#xvsUpdated method directly.

Impact

If a burn happens to the user. Even if it has enough xvs staked and waits a long time (longer then the needed waiting period) it will not get a new Prime token.

Proof of Concept

The following POC is written in two parts. One part shows what happens if the user after token burn does call xvsUpdate the other what happens if it does not.

it("M03 xvsUpdate will restart the waiting period after burn", async () => {
    const user = user1;

    // stake and wait some time to get the Prime token
    await xvs.connect(user).approve(xvsVault.address, bigNumber18.mul(10000));
    await xvsVault.connect(user).deposit(xvs.address, 0, bigNumber18.mul(10000));
    
    await mine(90 * 24 * 60 * 60);
    await prime.connect(user).claim();

    let token = await prime.tokens(user1.getAddress());
    expect(token.exists).to.be.equal(true);

    // burn the prime token
    await prime.burn(user1.getAddress());
    token = await prime.tokens(user1.getAddress());
    expect(token.exists).to.be.equal(false);

    let xvsBalance = await xvsVault.connect(user).getUserInfo (xvs.address, 0, user1.getAddress());
    let stake = await prime.stakedAt(user.getAddress());

    // !! without the following line the call to claim() will revert even if we have enough xvs staken !!
    // we need to call this line for waiting period time to start counting
    // this should happen automatically if user has enough xvs token staked after burn
    await prime.xvsUpdated(user.getAddress()); 
    console.log("Amount staked:", xvsBalance.amount.toString(), "staked:",stake.toString());

    await mine(91 * 24 * 60 * 60);
    await prime.connect(user1).claim();

    token = await prime.tokens(user1.getAddress());
    expect(token.exists).to.be.equal(true);
});

it("M03 without xvsUpdate will get a revert on claim even if we wait a long long time", async () => {
    const user = user1;

    // stake and wait some time to get the Prime token
    await xvs.connect(user).approve(xvsVault.address, bigNumber18.mul(10000));
    await xvsVault.connect(user).deposit(xvs.address, 0, bigNumber18.mul(10000));
    
    await mine(90 * 24 * 60 * 60);
    await prime.connect(user).claim();

    let token = await prime.tokens(user1.getAddress());
    expect(token.exists).to.be.equal(true);

    // burn the prime token
    await prime.burn(user1.getAddress());
    token = await prime.tokens(user1.getAddress());
    expect(token.exists).to.be.equal(false);

    let xvsBalance = await xvsVault.connect(user).getUserInfo (xvs.address, 0, user1.getAddress());
    let stake = await prime.stakedAt(user.getAddress());

    
    console.log("Amount staked:", xvsBalance.amount.toString(), "staked:",stake.toString());

    // wait longer then needed
    await mine((100 + 91) * 24 * 60 * 60);
    await expect(prime.connect(user).claim()).to.be.revertedWithCustomError(prime, "IneligibleToClaim");
    token = await prime.tokens(user1.getAddress());
    expect(token.exists).to.be.equal(false);
});

run it with npx hardhat test tests/hardhat/Prime/Prime.ts --grep "M03"

and will get this output :

  PrimeScenario Token
    mint and burn
Amount staked: 10000000000000000000000 staked: 0
      ✔ M03 xvsUpdate will restart the waiting period after burn (567ms)
Amount staked: 10000000000000000000000 staked: 0
      ✔ M03 without xvsUpdate will get a revert on claim even if we wait a long long time (395ms)


  2 passing (8s)

Tools Used

Manual review

Recommended Mitigation Steps

When burning the token with burn restart the waiting period for the user by invoking xvsUpdate and it will take care of all needed accounting.

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);
+   xvsUpdated(user);
+
    emit Burn(user);
}

Assessed type

Invalid Validation