VTVL contest - rajatbeladiya's results

Lines of code

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

Vulnerability details

Impact

maxSupply is used to limit the totalSupply after initial supply but here mint() will continue to mint even if mintableSupply passed.

Proof of Concept

1. Alice deployed token with initialSupply = 100 and maxSupply = 200
2. Alice call mint() with 200 tokens amount, mintableSupply will be 0 and it will mint 200 tokens
3. Alice again call mint() with 500 tokens amount,
   • if(mintableSupply > 0) will be false because mintableSupply is 0
   • Mints 500 tokens direct

Tools Used

Manual Analysis

Recommended Mitigation Steps

• _mint() only if mintableSupply > 0

  if(mintableSupply > 0) {
    require(account != address(0), "INVALID_ADDRESS");
    mintableSupply -= amount;
    _mint(account, amount);
  }
  

Lines of code

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

Vulnerability details

Impact

if admin revoked any recipient’s claim, admin can not create claim for the same recipient because startTimestamp is not updated to initial value on revoke claim. There will be a need to create a claim again for any reason like, 1. mistakenly revoked claim 2. Wrong info provided to claim 3. new vesting period starts, etc.

Proof of Concept

1. Alice create claim for Bob
2. Alice revoke claim of Bob
   • On revokeClaim(), claim’s isActive will be false, but startTimestamp will be remain as it is
   • https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L418-L437
3. Alice try to create claim for Bob but claim will not create because it has modifier hasNoClaim() which is checked for claim should not active and it checks for require(_claim.startTimestamp == 0, "CLAIM_ALREADY_EXISTS");

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

Tools Used

Manual Analysis

Recommended Mitigation Steps

Update startTimestamp to 0 on revokeClaim()

1. Unused imported Ownable contract

   • remove unused code

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

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

2. Unlocked pragma

   • recommend to use locked pragma because unlocked pragma uses, meaning compiler will use the specified version or above. It's good practice to use specific solidity version to know compiler bug fixes and optimisations were enabled at the time of compiling the contract.

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

3. Storage could be changed to memory

   • https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L197
   • https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L207
   • https://github.com/code-423n4/2022-09-vtvl/blob/f68b7f3e61dad0d873b5b5a1e8126b839afeab5f/contracts/VTVLVesting.sol#L216

4. withdrawAdmin() function could be declared as external

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