code-423n4 / 2023-09-venus-findings

4 stars 4 forks source link

Anyone can burn issued Soulbound Revocable token for a user if the user doesn’t have the minimum staked amount of XVS at the moment of issuance #271

Open c4-submissions opened 1 year ago

c4-submissions commented 1 year ago

Lines of code

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

Vulnerability details

Impact

The issue(bool isIrrevocable, address[] calldata users) method can be called by Governance to mint Soulbound tokens to users. In the case where the user has been issued a Revocable Soulbound token, but at the current moment has staked less than MINIMUM_STAKED_XVS, their token can get burned by anyone. An attacker can call xvsUpdated(address user), before the user has a chance to increase their stake and this will lead to the user's token getting burned.

Proof of Concept

The issue(bool isIrrevocable, address[] calldata users) method does not take into account user's staked XVS and xvsUpdated(address user) can be called by anyone.

    it("Issue: burn issued token", async () => {
      await prime.issue(false, [user1.getAddress(), user2.getAddress()]);

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

      await prime.connect(user2).xvsUpdated(user1.getAddress());

      const tokenAfterBurn = await prime.tokens(user1.getAddress());
      expect(tokenAfterBurn.exists).to.be.equal(false);
    });

Steps

  1. Governance issues Soulbound tokens to Alice, Bob and Elvis, each of them having no staked XVS at the current moment.
  2. John sees that Alice has been issued an Soulbound token and calls xvsUpdated(alice_address) and deletes her Soulbound Revocable Token.

Tools Used

Manual Analysis

Recommended Mitigation Steps

There are two possible mitigation steps and I will advise to implement both of them:

  1. Take into account user's staked XVS when issuing Soulbound Revocable tokens

if (_xvsBalanceOfUser(user) < MINIMUM_STAKED_SVS) revert NotEnoughXVSStaked();

  1. Implement access control for xvsUpdated(address user) so it can be called only by XVSVault.sol

if (msg.sender != xvsVault) revert NotAuthorized();

Assessed type

Invalid Validation

c4-pre-sort commented 1 year ago

0xRobocop marked the issue as duplicate of #485

c4-judge commented 11 months ago

fatherGoose1 changed the severity to QA (Quality Assurance)

c4-judge commented 11 months ago

fatherGoose1 marked the issue as grade-b