VTVL contest - cccz's results

Lines of code

https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/token/VariableSupplyERC20Token.sol#L35-L46

Vulnerability details

Impact

In the VariableSupplyERC20Token contract, when mintableSupply is set, the total number of tokens minted should be less than mintableSupply. However, in practice, since mintableSupply subtracts the number of tokens minted, when mintableSupply is 0, this limit is bypassed, resulting in an unlimited number of tokens that can be minted.

    function mint(address account, uint256 amount) public onlyAdmin {
        require(account != address(0), "INVALID_ADDRESS");
        // If we're using maxSupply, we need to make sure we respect it
        // mintableSupply = 0 means mint at will
        if(mintableSupply > 0) {
            require(amount <= mintableSupply, "INVALID_AMOUNT");
            // We need to reduce the amount only if we're using the limit, if not just leave it be
            mintableSupply -= amount;
        }
        _mint(account, amount);
    }

Consider the following scenario. The admin sets mintableSupply = 5000, indicating that the total supply of tokens is 5000. The admin then minted 5000 tokens, and mintableSupply = 0. Since mintableSupply = 0, the mint function no longer requires amount < mintableSupply, and the admin can mint unlimited tokens.

Proof of Concept

https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/token/VariableSupplyERC20Token.sol#L35-L46

Tools Used

None

Recommended Mitigation Steps

Consider using _totalSupply to limit the total supply of tokens. https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/ERC20.sol#L264

Lines of code

https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L446-L451

Vulnerability details

Impact

The withdrawOtherToken function is used to withdraw non-tokenAddress tokens from the contract, but not ETH. if the user accidentally sends ETH to the contract, ETH will be frozen.

Proof of Concept

https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L446-L451

Tools Used

None

Recommended Mitigation Steps

Consider adding the withdrawETH function to withdraw ETH from the contract