VTVL contest - 0xDecorativePineapple's results

Lines of code

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

Vulnerability details

Impact

It has been identified that a user can mint more tokens than the maxSupply_ in the VariableSupplyERC20Token.sol smart contract. As noted in the comments of the smart contract: // If we're using maxSupply, we need to make sure we respect it

So, if the constructor is called with 100 as the maxSupply_ value, the maxTokens that can be minted are 100. However, the check in the mint() function does not correctly enforce this condition.

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);
    }

If the mintableSupply becomes 0 after the subtraction at L:41 mintableSupply -= amount; , the mint operation at L:45 _mint(account, amount); will continue normally.

Proof of Concept

Bellow is a forge test noting the above issue.

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Test.sol";
import "../src/token/VariableSupplyERC20Token.sol";

contract CounterTest is Test {
    address owner = address(1);
    VariableSupplyERC20Token variableSupplyERC20Token;

    function setUp() public {
        vm.startPrank(owner);
        //deploy the VariableSupplyERC20Token smart contract with 100 as the max supply
        variableSupplyERC20Token = new VariableSupplyERC20Token("name1", "symbol1", 0, 100);
        vm.stopPrank();
    }

    function test1() public{
        vm.startPrank(owner);
        //log the mintableSupply
        console.log(variableSupplyERC20Token.mintableSupply());
        //mint 100 tokens to the user
        variableSupplyERC20Token.mint(owner, 100);

        //log the token balance of the user
        console.log(variableSupplyERC20Token.balanceOf(owner));

        //log once againt the mintableSupply in order to make sure that is zero
        console.log(variableSupplyERC20Token.mintableSupply());

        //mint 200 more tokens to the user
        variableSupplyERC20Token.mint(owner, 200);

        //log once again the token balance of the user
        console.log(variableSupplyERC20Token.balanceOf(owner));

        //log once againt the mintableSupply in order to make sure that is zero
        console.log(variableSupplyERC20Token.mintableSupply());
        vm.stopPrank();
    }
}

Tools Used

Manual Code Review

Recommended Mitigation Steps

It is recommended to add a require statement in the beginning of the mint() function to ensure that the mintableSupply is > 0 or mintAtWill is true. The mintAtWill variable can be set to true if the maxSupply_ == 0 .

Lines of code

https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L388 https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L407 https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L450

Vulnerability details

Impact

The VTVL documentation clearly notes that: VTVLVesting contract supports any ERC20 token, so any other token can be substituted for this one. But, it doesn't appear to support rebasing/deflationary/inflationary tokens whose balance changes during transfers or over time. The necessary checks include at least verifying the amount of tokens transferred to contracts before and after the actual transfer to infer any fees/interest.

Proof of Concept

VTVLVesting.sol#L388 VTVLVesting.sol#L407 VTVLVesting.sol#L450

Tools Used

Manual Code Review

Recommended Mitigation Steps

• Ensure that to check previous balance/after balance equals to amount for any rebasing/inflation/deflation
• Add support in contracts for such tokens before accepting user-supplied tokens
• Consider supporting deflationary / rebasing / etc tokens by extra checking the balances before/after or strictly inform your users not to use such tokens if they don’t want to lose them.

Rounding in _baseVestedAmount function leads to zero amount available for withdraw after some time passes

Impact

It has been identified that is possible for a user to not be able to withdraw any funds, even he has an active claim and some time has passed through the linear vesting period.

This happens due to the rounding in the _baseVestedAmount function.

The available withdrawn amount is 0 when linearVestAmount * truncatedCurrentVestingDurationSecs < finalVestingDurationSecs

Suppose that an administrator creates claim for userA with the values:

address _recipient : user A 
uint40 _startTimestamp : uint40(10)
uint40 _endTimestamp: uint40(545443020800)
uint40 _cliffReleaseTimestamp: 0
uint40 _releaseIntervalSecs: 10
uint112 _linearVestAmount: uint112(3965)
uint112 _cliffAmount: 0

After 9000 seconds the amount that is available to be withdrawn will be 0. This is inconvenient for the users as they try to withdraw the funds and the transaction reverts.

Proof of Concept

Bellow is a foundry test noting the issue

pragma solidity ^0.8.13;

import "forge-std/Test.sol";
import "../src/test/TestERC20Token.sol";
import "../src/VTVLVesting.sol";


contract CounterTest is Test {
    address owner = address(1);
    address user1 = address(2);
    address user2 = address(3);
    TestERC20Token testERC20Token;
    VTVLVesting vtvlVesting;

    function setUp() public {
        //start a prank as the owner
        vm.startPrank(owner);
        //deploy a test token with 4000*10**18 initial supply
        testERC20Token = new TestERC20Token("testName", "testSymbol", 4000);

        //deploy the vtvlVesting contract
        vtvlVesting = new VTVLVesting(testERC20Token);

        //transfer 200 * 10 ** 18 tokens to the vtvlVesting smart contract
        testERC20Token.transfer(address(vtvlVesting), 900 * 10 ** 18);
  
        vm.stopPrank();       
    }
    
    function testVerifyZeroWithdraw() public {
        vm.prank(owner);

        vtvlVesting.createClaim(user1, uint40(10), uint40(545443020800), 0, 10, uint112(3965), 0);

        skip(9000);
        console.log(vtvlVesting.claimableAmount(user1));

        vm.startPrank(user1);
        vtvlVesting.withdraw();

        vm.stopPrank();
    }
}