VTVL contest - adriro's results

Lines of code

https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/token/VariableSupplyERC20Token.sol#L36

Vulnerability details

Impact

VariableSupplyERC20Token allows to set a "maxSupply_" during construction which should set the maximum amount that an admin can mint.

@param maxSupply_ - What's the maximum supply. The contract won't allow minting over this amount. Set to 0 for no limit.

This variable is later used in the mint() function to check if there's enough amount left to mint more tokens. However, if the minted amount(s) (ie one or more calls to mint) reduce this amount to 0, then the behavior changes to the "no limit" mode which will allow any amount to be minted later on.

Giving this a medium risk since this function is admin restricted.

Proof of Concept

1. Create a token with maxSupply > 0.
2. Call mint so that this amount gets reduced to exactly 0.
3. After this, any amount can be minted.

Here's a small test to reproduce the issue:

describe("VariableSupplyERC20Token", async function () {
  it("should not mint more than max supply", async () => {
    const [admin, bob, alice] = await ethers.getSigners();

     const maxSupply = 100;
     const initialSupply = 0;

     const VariableSupplyERC20Token = await ethers.getContractFactory("VariableSupplyERC20Token", admin);
     const token = await VariableSupplyERC20Token.deploy("Broken", "BRO", initialSupply, maxSupply);
     await token.deployed();

     expect(await token.isAdmin(admin.address)).to.be.true;

     // we mint so that mintableSupply is reduced to exactly 0, which resets the
     // "max supply" behaviour and allows to mint any amount
     await (await token.mint(bob.address, maxSupply / 2)).wait();
     await (await token.mint(bob.address, maxSupply / 2)).wait();

     await expect(token.mint(alice.address, 999)).to.be.revertedWith("INVALID_AMOUNT");
  });
});

Recommended Mitigation Steps

Keep track of the "no limit" mode in a separate variable instead of using the logic of mintableSupply == 0.

Simplify finalVestedAmount in revokeClaim

finalVestedAmount can be simplified to the sum of linear + cliff amounts:

uint112 finalVestAmt = _claim.linearVestAmount + _claim.cliffAmount;

Since revokeClaim operates on active claims, the final vested amount should be equivalent to adding both amounts.

Since this is only usage of finalVestedAmount the function can be removed.

https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L422