Venus Prime - mahdirostami's results

Lines of code

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

Vulnerability details

impact

Users can stake XVS tokens, and after that the protocol might decide to issue irreversible tokens for them. However, there's an issue with the “issue” function – it doesn't delete the stakedAt information for users when minting these irreversible tokens. Consequently, if the protocol decides to burn their tokens, users can claim their revocable tokens immediately, without any waiting period.

• some users could bypass the waiting time that cause earning more rewards rather than other users.

Proof of Concept

Test:

    it("audit stakeAt", async () => {
      await xvs.connect(user1).approve(xvsVault.address, bigNumber18.mul(10000));
      await xvsVault.connect(user1).deposit(xvs.address, 0, bigNumber18.mul(10000));

      let stake = await prime.stakedAt(user1.getAddress());
      expect(stake).be.gt(0);
      console.log("user's 'stakeAt' when he deposits: ", stake.toString());

      console.log("protocol decide to issue irrevocable token.");
      await prime.issue(true, [user1.getAddress()]);
      let token = await prime.tokens(user1.getAddress());
      expect(token.exists).to.be.equal(true);
      expect(token.isIrrevocable).to.be.equal(true);
      console.log("issued.");
      stake = await prime.stakedAt(user1.getAddress());
      expect(stake).be.gt(0);
      console.log("user's 'stakeAt' after issuing irrevocable token: ", stake.toString());
      await mine(90 * 24 * 60 * 60);
      console.log("after some days protocol decide to burn user's irrevocable token.");
      await prime.burn(user1.getAddress());
      token = await prime.tokens(user1.getAddress());
      expect(token.isIrrevocable).to.be.equal(false);
      expect(token.exists).to.be.equal(false);
      console.log("burned.");
      stake = await prime.stakedAt(user1.getAddress());
      console.log("user's 'stakeAt' after burning irrevocable token: ", stake.toString());
      console.log("user mints token without waiting 90 days.");
      await expect(prime.connect(user1).claim()).to.be.not.reverted;
      token = await prime.tokens(user1.getAddress());
      expect(token.isIrrevocable).to.be.equal(false);
      expect(token.exists).to.be.equal(true);
      console.log("minted.");
      stake = await prime.stakedAt(user1.getAddress());
      expect(stake).be.equal(0);
      console.log("user's 'stakeAt' after minting: ", stake.toString());
    });

Output :

user's 'stakeAt' when he deposits:  1696227333
protocol decide to issue irrevocable token.
issued.
user's 'stakeAt' after issuing irrevocable token:  1696227333
after some days protocol decide to burn user's irrevocable token.
burned.
user's 'stakeAt' after burning irrevocable token:  1696227333
user mints token without waiting 90 days.
minted.
user's 'stakeAt' after minting:  0
      ✔ audit stakeAt (517ms)


  1 passing (6s)

Tools Used

manual review

Recommended Mitigation Steps

delete stakedAt[users[i]]; after minting irrevocable token.

        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]); 
+                    delete stakedAt[users[i]];
                }

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

• 3 Users with same score, but one of them gain rewards.(an attacker or normal user could do this.)

Proof of Concept

Protocol decide to add new market and after adding new market, Protocol must call updateScores. If there is a gap between these two functions, in a situation where three users have the same score, but one of them updates their status before the others. This user gains an advantage by being the first to update, and gain all rewards.

Test:

it("addMarket", async () => {
        bnb.transfer(user3.getAddress(), bigNumber18.mul(100));
        bnb.transfer(user4.getAddress(), bigNumber18.mul(100));
        bnb.transfer(user5.getAddress(), bigNumber18.mul(100));

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

        await bnb.connect(user3).approve(vbnb.address, bigNumber18.mul(90));
        await vbnb.connect(user3).mint(bigNumber18.mul(90));
        await bnb.connect(user4).approve(vbnb.address, bigNumber18.mul(90));
        await vbnb.connect(user4).mint(bigNumber18.mul(90));
        await bnb.connect(user5).approve(vbnb.address, bigNumber18.mul(90));
        await vbnb.connect(user5).mint(bigNumber18.mul(90));

        await xvs.connect(user3).approve(xvsVault.address, bigNumber18.mul(2000));
        await xvsVault.connect(user3).deposit(xvs.address, 0, bigNumber18.mul(2000));
        await xvs.connect(user4).approve(xvsVault.address, bigNumber18.mul(2000));
        await xvsVault.connect(user4).deposit(xvs.address, 0, bigNumber18.mul(2000));
        await xvs.connect(user5).approve(xvsVault.address, bigNumber18.mul(2000));
        await xvsVault.connect(user5).deposit(xvs.address, 0, bigNumber18.mul(2000));

        await prime.issue(false, [user3.getAddress(), user4.getAddress(), user5.getAddress()]);
        await prime.addMarket(vbnb.address, bigNumber18.mul(1), bigNumber18.mul(1));
        console.log("market added");
        console.log("userA update himself");
        await prime.accrueInterestAndUpdateScore(user3.getAddress(), vbnb.address);
        let market = await prime.markets(vbnb.address);
        let interest3 = await prime.interests(vbnb.address, user3.getAddress());
        console.log("userA score", interest3.score.toString());
        console.log("userA rewardIndex", interest3.rewardIndex.toString());
        console.log("userA accrued", interest3.accrued.toString());
        console.log("market score", market.sumOfMembersScore.toString());
        console.log("market rewardIndex", market.rewardIndex.toString());
        console.log("--------------------------------------------------------");
        console.log("market gain some reward");
        await protocolShareReserve.getUnreleasedFunds.returns(103687);
        await prime.accrueInterest(vbnb.address);
        console.log("--------------------------------------------------------");
        console.log("Protocol decide to update 3 users");
        await prime.updateScores([user3.getAddress(), user4.getAddress(), user5.getAddress()]);
        interest3 = await prime.interests(vbnb.address, user3.getAddress());
        let interest4 = await prime.interests(vbnb.address, user4.getAddress());
        let interest5 = await prime.interests(vbnb.address, user5.getAddress());
        market = await prime.markets(vbnb.address);

        console.log("userA score", interest3.score.toString());
        console.log("userB score", interest4.score.toString());
        console.log("userC score", interest5.score.toString());

        console.log("userA rewardIndex", interest3.rewardIndex.toString());
        console.log("userB rewardIndex", interest4.rewardIndex.toString());
        console.log("userC rewardIndex", interest5.rewardIndex.toString());

        console.log("userA accrued", interest3.accrued.toString());
        console.log("userB accrued", interest4.accrued.toString());
        console.log("userC accrued", interest5.accrued.toString());

        console.log("market score", market.sumOfMembersScore.toString());
        console.log("market rewardIndex", market.rewardIndex.toString());
        console.log("-----------------------------------------------------------");
      });
    });

Output:

market added
userA update himself
userA score 200000000000000029058
userA rewardIndex 0
userA accrued 0
market score 200000000000000029058
market rewardIndex 0
--------------------------------------------------------
market gain some reward
--------------------------------------------------------
Protocol decide to update 3 users
userA score 200000000000000029058
userB score 200000000000000029058
userC score 200000000000000029058
userA rewardIndex 518
userB rewardIndex 518
userC rewardIndex 518
userA accrued 103600
userB accrued 0
userC accrued 0
market score 600000000000000087174
market rewardIndex 518
-----------------------------------------------------------
        ✔ addMarket (1690ms)


  1 passing (8s)

Tools Used

Manual review

Recommended Mitigation Steps

call updateScores in addMarket function.

Assessed type

Other