VTVL contest - cryptonue's results

Lines of code

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

Vulnerability details

Impact

By definition and comments inside the contract, the VariableSupplyERC20Token.sol will have option to set it as limited supply or unlimited supply. This decision is made on constructor parameter (#L21).

If the token want to be limited supply, then the maxSupply_ need to be set > 0, if it's 0 then it's unlimited supply. on line #L28, the mintableSupply is set with this maxSupply_.

Proof of Concept

The problem is on the mint() function, there is a flaw.

The mint function is not respecting the mintableSupply. On line #43, the mintableSupply is decreased by mint amount. This means, at some point, the mintableSupply can be 0, thus the #L40-#L44 is not being called anymore, so any mint call, will directly _mint() it.

This mint function can be a potential factor of miscalculation of token supply, affecting Vesting calculation for user, or further usage of this token.

Recommended Mitigation Steps

We need to store mintedToken into a state variable, and increment it when minting happen, moreover we should make mintableSupply as immutable, and we use it to check if amount + mintedToken is <= mintableSupply

uint256 mintedToken = 0;

function mint(address account, uint256 amount) public onlyAdmin {
    require(account != address(0), "INVALID_ADDRESS");
    
    if(mintableSupply > 0) {
      require(amount + mintedToken <= mintableSupply, "INVALID_AMOUNT");
    }

    mintedToken += amount;
    _mint(account, amount);
}