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

4 stars 4 forks source link

Prime bulk issuance of revocable token griefing #186

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/b11d9ef9db8237678567e66759003138f2368d23/contracts/Tokens/Prime/Prime.sol#L349-L357

Vulnerability details

Impact

When performing the bulk issuing of revocable Prime tokens, if a member of the address set already has a Prime token, the transaction reverts rather than gracefully handling to allow the other members of the set to receive their tokens.

A new transaction without the offending address in the set must be submitted, which is an inconvenience in both time and cost, compounded if there's also governance aspect surrounding the update.

Description

Prime.issue directly issues of either Irrevocable or revocable Prime tokens to a list of addresses.

Only after a user has their Prime token minted do they accrue interest, providing a financial incentive to claim their token as soon as possible.

Prime.issue contains access control, meaning it may be invoked via governance process, usually a larger time delay than a centralised admin.

When a Prime token is minted, if the address already possesses one then the custom error IneligibleToClaim is thrown.

Prime.issue of revocable assumes none of the given addresses already possess a Prime token.

Proof of Concept

Add the below test to the mint and burn describe in tests/hardhat/integration/index.ts and you have the issue failing with the custom error IneligibleToClaim in _mint:

    it("bulk issue grief for revocable Prime token", async () => {
      const userOneAddress = user1.getAddress();
      const userTwoAddress = user2.getAddress();

      expect((await prime.tokens(userOneAddress)).exists).to.be.false;
      expect((await prime.tokens(userTwoAddress)).exists).to.be.false;

      // User1 stakes then claims their revocable Prime token
      await xvs.connect(user1).approve(xvsVault.address, bigNumber18.mul(10000));
      await xvsVault.connect(user1).deposit(xvs.address, 0, bigNumber18.mul(10000));
      await prime.stakedAt(userOneAddress);
      await mine(90 * 24 * 60 * 60);
      await prime.connect(user1).claim()
      expect((await prime.tokens(userOneAddress)).exists).to.be.true;

      // Transaction reverts (i.e. no tokens issued) as user1 already has a revocable token
      await expect(prime.issue(false, [userOneAddress, userTwoAddress])).to.be.revertedWithCustomError(prime, "IneligibleToClaim");
    })

Tools Used

Webstorm

Recommended Mitigation Steps

Update the code to check whether the address already possesses a Prime token, if true then skips over as they have already been issued one, by some mechanism.

Prime.sol#L350

+               if(!tokens[users[i]].exists) {_mint(false, users[i]);}
-                _mint(false, users[i]);

Assessed type

Invalid Validation

0xRobocop commented 1 year ago

See #187

c4-pre-sort commented 1 year ago

0xRobocop marked the issue as low quality report

c4-judge commented 11 months ago

fatherGoose1 marked the issue as unsatisfactory: Invalid

c4-judge commented 11 months ago

fatherGoose1 changed the severity to QA (Quality Assurance)

fatherGoose1 commented 11 months ago

QA. The griefing is easily remedied by passing a critical governance proposal. The griefer will have made little impact after having to have staked for >90 days.

c4-judge commented 11 months ago

fatherGoose1 marked the issue as grade-b