VTVL contest - TomJ's results

Lines of code

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

Vulnerability details

Impact

It is not ideal for admins to be able to withdraw any vested amount that is ALREADY claimable by vesting recipients to keep vesting recipient's (or investors or employees) incentives alligned with admins project. The point I want to say is that, current revoke function is possible for admins to revoke the claim even AFTER the vested amount schedule is reached. This means that recipients technically lost their funds that was available, if they didn't withdraw before it was revoked by admins and admins can now withdraw that amount for themselves instead. This is too much power given to admin and not ideal for vesting token process.

Proof of Concept

I will explain in more details by referencing some of VTVL team documents from the contest page.

At the VTVL contest page it is stated as

due of the nature of vesting, e.g. employee token vesting, our vesting 
contract is deliberately designed to allow admin revocation in the circumstances of early 
employment termination (before the end of vesting period specified).

If the project intention is to revoke the claim for employees who is terminated, then current revoke function has too much power than it should be. Because the current revokeClaim function in VTVLVesting.sol contract can revoke the claim even though there is already valid vested amount claimable by recipients, meaning that if recipients didn't withdraw their claimable vested amount before the admin execute revokeClaim function, they will loss their vested amount. This revokeClaim function should instead just close the remaining future vesting schedule and allow recipients to withdraw any vested amount token that already reached the schedule.

Vesting recipients
If an user has a valid claim associated to their address, they have the ability to withdraw the 
amount that's claimable at the moment they make the claim. No one other than the designated 
vesting recipient (not even the admin) can withdraw on an existing claim - but the admin can 
revoke the claim.

Also above statement of No one other than the designated vesting recipient (not even the admin) can withdraw on an existing claim about vesting recipients is not true because admin can technically withdraw on an existing claim because admin can revoke claim anytime they want and if the vested amount is not yet withdrawn by vesting recipients, this amount will be deducted from numTokensReservedForVesting variable, which means that recipients can no longer withdraw the vested amount and admins can withdraw instead, which is technically same as admin withdraw on an existing claim.

Tools Used

Manual Analysis

Recommended Mitigation Steps

It is hard to implement above logic without changing too much of existing code but below example might be the simplest way. For revokeClaim function, keep line 419-420, and swap remaining line 421-436 with below code. Below code is checking whether current time is not passed the _claim.endTimestamp and if there is any vested amount that is not yet withdrawn by recipient. If there is, this amount will sent to recipient and remaining future amount will be back with the VTVLVesting contract and close the claim.

    // No point in revoking after the vesting schedule is finished
    require(uint40(block.timestamp) < _claim.endTimestamp, "VESTING_ENDED");

    // Calculate what the claim should finally vest to
    uint112 finalVestAmt = finalVestedAmount(_recipient);

    // Calculating vested amount of recipient at current time
    uint112 currentVestAmt = vestedAmount(_recipient, uint40(block.timestamp));

    // Check whether recipient has any claimable vested amount that is not withdrawn yet
    if (currentVestAmt > _claim.amountWithdrawn) {

        // Vested amount that recipient can claim but not yet withdrawn
        uint112 amountRemainingForRecipient = currentVestAmt - _claim.amountWithdrawn;

        // Vested amount that was scheduled in future but now will be revoked
        uint112 amountRemaining = finalVestAmt - currentVestAmt;

        // Deactivate the claim, and release the appropriate amount of tokens
        _claim.isActive = false;
        numTokensReservedForVesting -= amountRemaining;

        // Send already claimable vested amount that is not withdrawn to recipient and close the claim
        tokenAddress.safeTransfer(_recipient, amountRemainingForRecipient);
        emit Claimed(_recipient, amountRemainingForRecipient);

    } else {

        uint112 amountRemaining = finalVestAmt - _claim.amountWithdrawn;
        _claim.isActive = false; 
        numTokensReservedForVesting -= amountRemaining;

    }

    emit ClaimRevoked(_recipient, amountRemaining, uint40(block.timestamp), _claim);

However this might now open to new vulnerability such as recipient selling their future claimable vesting token by creating malicious contract and making that contract as an recipient. This can be prevented by checking whether vesting recipients is EOA and not a contract. Reference: https://blog.openzeppelin.com/bypassing-smart-contract-timelocks/

Lines of code

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

Vulnerability details

Impact

There is no check whether _startTimestamp and _endTimestamp is greater than block.timestamp at VTVLVesting.sol _createClaimUnchecked function. Therefore it is possible for administrators to accidentally create vesting schedule that starts and ends in the past without noticing it. When administrators does this and this transaction goes through, then the vesting recipients can withdraw their entire vest amount which is not what administrators intended to do. Add require check that force _startTimestamp to be greater than block.timestamp.

Team comments as below on line 260

// -> Conclusion: we want to allow this, for founders that might have forgotten to add some users, or to avoid issues with 
transactions not going through because of discoordination between block.timestamp and sender's local time

However this is not an issue by adding require(_startTimestamp > uint40(block.timestamp)) since this will revert transaction if _startTimestamp is less than block.timestamp so administrators can simply try again with correct time. On the other hand, it is more dangerous to not include this check because transaction will simply succeed even though _startTimestamp is set to past which means that there is a chance of administrators not noticing this.

Proof of Concept

1. Admin creates new vesting schedule using createClaim function. However admin mistakenly set _startTimestamp and _endTimestamp in the past.
2. Since there is no check of require(_startTimestamp > uint40(block.timestamp)), this transaction is valid and claim is created.
3. Vesting recipients calls the withdraw function and receive entire vest amount.

Tools Used

Manual Analysis

Recommended Mitigation Steps

Add following check in VTVLVesting.sol:_createClaimUnchecked function.

require(_startTimestamp > uint40(block.timestamp), "INVALID_START_TIMESTAMP")

Table of Contents

Low Risk Issues

• Same address can't be assigned as Vesting Recipients even though vesting schedule is finished
• Duplicate Context inheritance

Non-critical Issues

• Use fixed compiler versions instead of floating version
• Event is Missing Indexed Fields

 

Low Risk Issues

Same address can't be assigned as Vesting Recipients even though vesting schedule is finished

Issue

There might be an occasion where administrators (founders) wants to deploy the token vesting with same token again. In currenct VTVLVesting.sol contract, it is not possible to assign same address as vesting recipients even though the vesting schedule is finished (and all of its vest amount is withdrawn) because claim.isActive is always true once a claim is created (It will only be false again if admin execute revokeClaim() before all of its vest amount is withdrawn). Therefore it might be ideal to set claim.isActive to false once all of the vest amount is withdrawn.

PoC

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

Mitigation

Add below code after line 384 to make claim not active after all its vest amount is withdrawn https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L364-L392

uint112 finalVestAmt = finalVestedAmount(_msgSender());
if ( usrClaim.amountWithdrawn == finalVestAmt ) {
    usrClaim.isActive = false; 
}

 

Duplicate Context inheritance

Issue

Context inheritance is unnecessary for VTVLVesting.sol since it is already inherited at AccessProtected.sol.

PoC

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

VTVLVesting.sol:
contract VTVLVesting is Context, AccessProtected {

https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/AccessProtected.sol#L11

AccessProtected.sol:
abstract contract AccessProtected is Context {

Mitigation

Remove Context from VTVLVesting.sol like shown in below example.

Example:
contract VTVLVesting is AccessProtected {

 

Non-critical Issues

Use fixed compiler versions instead of floating version

Issue

it is best practice to lock your pragma instead of using floating pragma. the use of floating pragma has a risk of accidentally get deployed using latest complier which may have higher risk of undiscovered bugs. Reference: https://consensys.github.io/smart-contract-best-practices/development-recommendations/solidity-specific/locking-pragmas/

PoC

./VariableSupplyERC20Token.sol:2:pragma solidity ^0.8.14;

Mitigation

I suggest to lock your pragma and aviod using floating pragma.

// bad
pragma solidity ^0.8.10;

// good
pragma solidity 0.8.10;

 

Event is Missing Indexed Fields

Issue

Each event should have 3 indexed fields if there are 3 or more fields.

PoC

./VTVLVesting.sol:59:    event ClaimCreated(address indexed _recipient, Claim _claim); 
./VTVLVesting.sol:64:    event Claimed(address indexed _recipient, uint112 _withdrawalAmount); 
./VTVLVesting.sol:69:    event ClaimRevoked(address indexed _recipient, uint112 _numTokensWithheld, uint256 revocationTimestamp, Claim _claim);
./VTVLVesting.sol:74:    event AdminWithdrawn(address indexed _recipient, uint112 _amountRequested);
./AccessProtected.sol:14:    event AdminAccessSet(address indexed _admin, bool _enabled);

Mitigation

Add up to 3 indexed fields when possible.

 

Table of Contents

Total of 10 issues found.

• Should Use Unchecked Block where Over/Underflow is not Possible
• Defined Variables Used Only Once
• X = X + Y costs less gass than X += Y for state variables
• Use require instead of &&
• Change Function Visibility Public to External
• Use Calldata instead of Memory for Read Only Function Parameters
• Boolean Comparisons
• Using Elements Smaller than 32 bytes (256 bits) Might Use More Gas
• ++i Costs Less Gas than i++
• Use Custom Errors to Save Gas

 

Should Use Unchecked Block where Over/Underflow is not Possible

Issue

Since Solidity 0.8.0, all arithmetic operations revert on overflow and underflow by default. However in places where overflow and underflow is not possible, it is better to use unchecked block to reduce the gas usage. Reference: https://docs.soliditylang.org/en/v0.8.15/control-structures.html#checked-or-unchecked-arithmetic

PoC

Total of 7 instances found.

1. _baseVestedAmount() of VTVLVesting.sol (line 167) https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L166-L167

Wrap line 167 with unchecked since underflow is not possible due to line 166 check
            if(_referenceTs > _claim.startTimestamp) {
                uint40 currentVestingDurationSecs = _referenceTs - _claim.startTimestamp; // How long since the start

2. _baseVestedAmount() of VTVLVesting.sol (line 170) https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L170

Wrap line 170 with unchecked since underflow is not possible due to line 262 check
262:        require(_startTimestamp < _endTimestamp, "INVALID_END_TIMESTAMP"); // _endTimestamp must be after _startTimestamp

170:                uint40 finalVestingDurationSecs = _claim.endTimestamp - _claim.startTimestamp; // length of the interval

3. _createClaimUnchecked() of VTVLVesting.sol (line 264) https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L264

Wrap line 264 with unchecked since underflow is not possible due to line 262 check
262:        require(_startTimestamp < _endTimestamp, "INVALID_END_TIMESTAMP"); // _endTimestamp must be after _startTimestamp
264:        require((_endTimestamp - _startTimestamp) % _releaseIntervalSecs == 0, "INVALID_INTERVAL_LENGTH");

4. withdraw() of VTVLVesting.sol (line 377) https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L374-L377

Wrap line 377 with unchecked since underflow is not possible due to line 374 check
374:        require(allowance > usrClaim.amountWithdrawn, "NOTHING_TO_WITHDRAW");
377:        uint112 amountRemaining = allowance - usrClaim.amountWithdrawn;

5. withdrawAdmin() of VTVLVesting.sol (line 400) https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L400

Wrap line 400 with unchecked since underflow is not possible due to line 295 check
400:        uint256 amountRemaining = tokenAddress.balanceOf(address(this)) - numTokensReservedForVesting;

295:        require(tokenAddress.balanceOf(address(this)) >= numTokensReservedForVesting + allocatedAmount, "INSUFFICIENT_BALANCE");

6. revokeClaim() of VTVLVesting.sol (line 429) https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L426-L429

Wrap line 429 with unchecked since underflow is not possible due to line 426 check
426:        require( _claim.amountWithdrawn < finalVestAmt, "NO_UNVESTED_AMOUNT");
429:        uint112 amountRemaining = finalVestAmt - _claim.amountWithdrawn;

7. mint() of VariableSupplyERC20Token.sol (line 43) https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/token/VariableSupplyERC20Token.sol#L41-L43

Wrap line 43 with unchecked since underflow is not possible due to line 41 check
41:            require(amount <= mintableSupply, "INVALID_AMOUNT");
43:            mintableSupply -= amount;

Mitigation

Wrap the code with uncheck like described in above PoC.

 

Defined Variables Used Only Once

Issue

Certain variables is defined even though they are used only once. Remove these unnecessary variables to save gas. For cases where it will reduce the readability, one can use comments to help describe what the code is doing.

PoC

Total of 2 instances found.

1. Remove '_claim' variable of vestedAmount() of VTVLVesting.sol https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L197-L198 Mitigation: Delete line 197 and replace line 198 with below code

        return _baseVestedAmount(claims[_recipient], _referenceTs);

2. Remove 'amountRemaining' variable of withdrawAdmin() of VTVLVesting.sol https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L400-L402 Mitigation: Delete line 400 and replace line 402 with below code

        require(tokenAddress.balanceOf(address(this)) - numTokensReservedForVesting >= _amountRequested, "INSUFFICIENT_BALANCE");

Mitigation

Don't define variable that is used only once. Details are listed on above PoC.

 

X = X + Y costs less gass than X += Y

Issue

X = X + Y costs less gass than X += Y for state variables

PoC

Total of 1 instance found.

1. numTokensReservedForVesting variable of VTVLVesting.sol https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L301

        numTokensReservedForVesting += allocatedAmount; // track the allocated amount

Change it to

        numTokensReservedForVesting = numTokensReservedForVesting + allocatedAmount; // track the allocated amount

It saves 8 gas Average gas before modificaiton: 167760 Average gas after modificaiton: 167752

 

Use require instead of &&

Issue

When there are multiple conditions in require statement, break down the require statement into multiple require statements instead of using && can save gas.

PoC

Total of 1 instance found.

./VTVLVesting.sol:344:        require(_startTimestamps.length == length &&
                                      _endTimestamps.length == length &&
                                      _cliffReleaseTimestamps.length == length &&
                                      _releaseIntervalsSecs.length == length &&
                                      _linearVestAmounts.length == length &&
                                      _cliffAmounts.length == length, 
                                      "ARRAY_LENGTH_MISMATCH"

Mitigation

Break down into several require statement. For example,

require(_startTimestamps.length == length);
require(_endTimestamps.length == length);
require(_cliffreleasetimestamps.length == length);
require(_releaseIntervalsSecs.length == length);
require(_linearVestAmounts.length == length);
require(_cliffAmounts.length == length, "ARRAY_LENGTH_MISMATCH");

 

Change Function Visibility Public to External

Issue

If the function is not called internally, it is cheaper to set your function visibility to external instead of public.

PoC

Total of 2 instances found.

1. VTVLVesting.sol:withdrawAdmin() https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L398

2. AccessProtected.sol:setAdmin() https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/AccessProtected.sol#L39

Mitigation

Change the function visibility to external.

 

Use Calldata instead of Memory for Read Only Function Parameters

Issue

It is cheaper gas to use calldata than memory if the function parameter is read only. Calldata is a non-modifiable, non-persistent area where function arguments are stored, and behaves mostly like memory. More details on following link. link: https://docs.soliditylang.org/en/v0.8.15/types.html#data-location

PoC

Total of 7 instances found.

./VTVLVesting.sol:334:        address[] memory _recipients, 
./VTVLVesting.sol:335:        uint40[] memory _startTimestamps, 
./VTVLVesting.sol:336:        uint40[] memory _endTimestamps, 
./VTVLVesting.sol:337:        uint40[] memory _cliffReleaseTimestamps, 
./VTVLVesting.sol:338:        uint40[] memory _releaseIntervalsSecs, 
./VTVLVesting.sol:339:        uint112[] memory _linearVestAmounts, 
./VTVLVesting.sol:340:        uint112[] memory _cliffAmounts) 

Mitigation

Change memory to calldata

 

Boolean Comparisons

Issue

It is more gas expensive to compare boolean with "variable == true" or "variable == false" than directly checking the returned boolean value.

PoC

Total of 1 instance found.

./VTVLVesting.sol:111:        require(_claim.isActive == true, "NO_ACTIVE_CLAIM");

Mitigation

Simply check by returned boolean value. Change it to

require(_claim.isActive, "NO_ACTIVE_CLAIM");

 

Using Elements Smaller than 32 bytes (256 bits) Might Use More Gas

Issue

Since EVM operates on 32 bytes at a time, if the element is smaller than that, the EVM must use more operations in order to reduce the elements from 32 bytes to specified size. Therefore it is more gas saving to use 32 bytes unless it is used in a struct or state variable in order to reduce storage slot.

Reference: https://docs.soliditylang.org/en/v0.8.15/internals/layout_in_storage.html

PoC

Total of 18 instances found.

./VTVLVesting.sol:27:    uint112 public numTokensReservedForVesting = 0;
./VTVLVesting.sol:64:    event Claimed(address indexed _recipient, uint112 _withdrawalAmount); 
./VTVLVesting.sol:74:    event AdminWithdrawn(address indexed _recipient, uint112 _amountRequested);
./VTVLVesting.sol:147:    function _baseVestedAmount(Claim memory _claim, uint40 _referenceTs) internal pure returns (uint112) {
./VTVLVesting.sol:148:        uint112 vestAmt = 0;
./VTVLVesting.sol:167:                uint40 currentVestingDurationSecs = _referenceTs - _claim.startTimestamp; // How long since the start
./VTVLVesting.sol:169:                uint40 truncatedCurrentVestingDurationSecs = (currentVestingDurationSecs / _claim.releaseIntervalSecs) * _claim.releaseIntervalSecs;
./VTVLVesting.sol:170:                uint40 finalVestingDurationSecs = _claim.endTimestamp - _claim.startTimestamp; // length of the interval
./VTVLVesting.sol:176:                uint112 linearVestAmount = _claim.linearVestAmount * truncatedCurrentVestingDurationSecs / finalVestingDurationSecs;
./VTVLVesting.sol:196:    function vestedAmount(address _recipient, uint40 _referenceTs) public view returns (uint112) {
./VTVLVesting.sol:206:    function finalVestedAmount(address _recipient) public view returns (uint112) {
./VTVLVesting.sol:215:    function claimableAmount(address _recipient) external view returns (uint112) {
./VTVLVesting.sol:292:        uint112 allocatedAmount = _cliffAmount + _linearVestAmount;
./VTVLVesting.sol:371:        uint112 allowance = vestedAmount(_msgSender(), uint40(block.timestamp));
./VTVLVesting.sol:377:        uint112 amountRemaining = allowance - usrClaim.amountWithdrawn;
./VTVLVesting.sol:398:    function withdrawAdmin(uint112 _amountRequested) public onlyAdmin {    
./VTVLVesting.sol:422:        uint112 finalVestAmt = finalVestedAmount(_recipient);
./VTVLVesting.sol:429:        uint112 amountRemaining = finalVestAmt - _claim.amountWithdrawn;

Mitigation

I suggest using uint256 instead of anything smaller and downcast where needed.

 

++i Costs Less Gas than i++

Issue

Prefix increments/decrements (++i or --i) costs cheaper gas than postfix increment/decrements (i++ or i--).

PoC

Total of 1 instance found.

./VTVLVesting.sol:353:        for (uint256 i = 0; i < length; i++) {

Mitigation

Change it to postfix increments/decrements. It saves 6 gas per loop. For example,

for (uint256 i = 0; i < length; ++i) {

 

Use Custom Errors to Save Gas

Issue

Custom errors from Solidity 0.8.4 are cheaper than revert strings. Details are explained here: https://blog.soliditylang.org/2021/04/21/custom-errors/

PoC

Total of 24 instances found.

./VTVLVesting.sol:82:        require(address(_tokenAddress) != address(0), "INVALID_ADDRESS");
./VTVLVesting.sol:107:        require(_claim.startTimestamp > 0, "NO_ACTIVE_CLAIM");
./VTVLVesting.sol:111:        require(_claim.isActive == true, "NO_ACTIVE_CLAIM");
./VTVLVesting.sol:129:        require(_claim.startTimestamp == 0, "CLAIM_ALREADY_EXISTS");
./VTVLVesting.sol:255:        require(_recipient != address(0), "INVALID_ADDRESS");
./VTVLVesting.sol:256:        require(_linearVestAmount + _cliffAmount > 0, "INVALID_VESTED_AMOUNT"); // Actually only one of linearvested/cliff amount must be 0, not necessarily both
./VTVLVesting.sol:257:        require(_startTimestamp > 0, "INVALID_START_TIMESTAMP");
./VTVLVesting.sol:262:        require(_startTimestamp < _endTimestamp, "INVALID_END_TIMESTAMP"); // _endTimestamp must be after _startTimestamp
./VTVLVesting.sol:263:        require(_releaseIntervalSecs > 0, "INVALID_RELEASE_INTERVAL");
./VTVLVesting.sol:264:        require((_endTimestamp - _startTimestamp) % _releaseIntervalSecs == 0, "INVALID_INTERVAL_LENGTH");
./VTVLVesting.sol:270:        require( 
                                  (
                                      _cliffReleaseTimestamp > 0 && 
                                      _cliffAmount > 0 && 
                                      _cliffReleaseTimestamp <= _startTimestamp
                                  ) || (
                                      _cliffReleaseTimestamp == 0 && 
                                      _cliffAmount == 0
                              ), "INVALID_CLIFF");
./VTVLVesting.sol:295:        require(tokenAddress.balanceOf(address(this)) >= numTokensReservedForVesting + allocatedAmount, "INSUFFICIENT_BALANCE");
./VTVLVesting.sol:344:        require(_startTimestamps.length == length &&
                                      _endTimestamps.length == length &&
                                      _cliffReleaseTimestamps.length == length &&
                                      _releaseIntervalsSecs.length == length &&
                                      _linearVestAmounts.length == length &&
                                      _cliffAmounts.length == length, 
                                      "ARRAY_LENGTH_MISMATCH"
                              );
./VTVLVesting.sol:374:        require(allowance > usrClaim.amountWithdrawn, "NOTHING_TO_WITHDRAW");
./VTVLVesting.sol:402:        require(amountRemaining >= _amountRequested, "INSUFFICIENT_BALANCE");
./VTVLVesting.sol:426:        require( _claim.amountWithdrawn < finalVestAmt, "NO_UNVESTED_AMOUNT");
./VTVLVesting.sol:447:        require(_otherTokenAddress != tokenAddress, "INVALID_TOKEN"); // tokenAddress address is already sure to be nonzero due to constructor
./VTVLVesting.sol:449:        require(bal > 0, "INSUFFICIENT_BALANCE");
./FullPremintERC20Token.sol:11:        require(supply_ > 0, "NO_ZERO_MINT");
./VariableSupplyERC20Token.sol:27:        require(initialSupply_ > 0 || maxSupply_ > 0, "INVALID_AMOUNT");
./VariableSupplyERC20Token.sol:37:        require(account != address(0), "INVALID_ADDRESS");
./VariableSupplyERC20Token.sol:41:            require(amount <= mintableSupply, "INVALID_AMOUNT");
./AccessProtected.sol:25:        require(_admins[_msgSender()], "ADMIN_ACCESS_REQUIRED");
./AccessProtected.sol:40:        require(admin != address(0), "INVALID_ADDRESS");

Mitigation

I suggest implementing custom errors to save gas.