Lines of code
<https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L147&gt;
<https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L196-L199&gt;
<https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L206-L209&gt;
<https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L215-L218&gt;
<https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L371&gt;
<https://github.com/code-423n4/2022-09-vtvl/blob/main/contracts/VTVLVesting.sol#L422&gt;

Vulnerability details

Impact

In contract VTVLVesting.sol, the multiplication in function _baseVestedAmount can overflow for big enough values oftruncatedCurrentVestingDurationSecsandlinearVestAmount. This means the claim could be successfully created by the admin, but could NEVER be revoked or fully withdrawn (it could only be withdrawn as far astruncatedCurrentVestingDurationSecs is small enough to not make the multiplication overflow.
In fact, ALL calls to function _baseVestedAmount passing in a value of _referenceTs greater than or equal to the claim’sendTimestamp would result in a revert. This affects functions vestedAmount, finalVestedAmount, claimableAmount, withdraw and revokeClaim.

Proof of Concept

The test below, shows how if we use a release interval of 1 second and a linear vesting span of 10 years, we can create a claim of 16,500,000 tokens, but if we then call function revokeClaim, the call reverts because the multiplication would overflow.

/* eslint-disable prettier/prettier */
import { expect } from "chai";
import { ethers } from "hardhat";
import Chance  from "chance";
// eslint-disable-next-line node/no-missing-import
import { VTVLVesting } from "../typechain";
import { BigNumber } from "ethers";
import { PANIC_CODES } from "@nomicfoundation/hardhat-chai-matchers/panic";
import "@nomicfoundation/hardhat-chai-matchers";

const chance = new Chance(43153); // Make sure we have a predictable seed for repeatability

const randomAddress = async () => {
  return await ethers.Wallet.createRandom().getAddress();
}

const createContractFactory = async () => await ethers.getContractFactory("VTVLVesting");
const deployVestingContract = async (tokenAddress?: string) => {
  const factory = await createContractFactory();
  // TODO: check if we need any checks that the token be valid, etc
  const contract = await factory.deploy(tokenAddress ?? await randomAddress());
  return contract;
}

const dateToTs = (date: Date|string) => ethers.BigNumber.from(Math.floor((date instanceof Date ? date : new Date(date)).getTime() / 1000));

const createPrefundedVestingContractBN = async (props: {tokenName: string, tokenSymbol: string, initialSupply: BigNumber}) => {
  const {
      tokenName, 
      tokenSymbol, 
      initialSupply, 
  } = props;

  // Create an example token
  const tokenContractFactory = await ethers.getContractFactory('TestERC20Token');
  // const initialSupply = ethers.utils.parseUnits(initialSupplyTokens.toString(), decimals);
  const tokenContract = await tokenContractFactory.deploy(tokenName, tokenSymbol, initialSupply);

  // Create an example vesting contract
  const vestingContract = await deployVestingContract(tokenContract.address);
  await vestingContract.deployed();

  expect(await vestingContract.tokenAddress()).to.be.equal(tokenContract.address);

  // Fund the vesting contract - transfer everything to the vesting contract (from the user)
  await tokenContract.transfer(vestingContract.address, await tokenContract.totalSupply());

  return {tokenContract, vestingContract};
}

type VestingContractType = VTVLVesting;

describe("My tests", async function () {

    it('overflow test', async () => {
        const recipientAddress = await randomAddress();
        const initialSupply = BigNumber.from("5100000000000000000000000000000000");
        const tokenName = "TOKEN";
        const tokenSymbol = "TK";
        const {vestingContract} = await createPrefundedVestingContractBN({tokenName, tokenSymbol, initialSupply});
        // Create and immediately revoke the claim
        const myLinearVestAmount = BigNumber.from("16500000000000000000000000");
        const myStartTimestamp = dateToTs('2012-02-01');
        const myEndTimestamp = dateToTs('2022-02-01');
        const myCliffReleaseTimestamp = dateToTs(new Date('2010-02-01'));
        const cliffAmount = BigNumber.from("16500000000000000000000000");
        const myReleaseIntervalSecs = 1;
        await vestingContract.createClaim(recipientAddress, myStartTimestamp, myEndTimestamp, myCliffReleaseTimestamp, myReleaseIntervalSecs, myLinearVestAmount, cliffAmount);
        await expect(vestingContract.vestedAmount(recipientAddress, myEndTimestamp)).to.be.revertedWithPanic(PANIC_CODES.ARITHMETIC_UNDER_OR_OVERFLOW);
        await expect(vestingContract.revokeClaim(recipientAddress)).to.be.revertedWithPanic(PANIC_CODES.ARITHMETIC_UNDER_OR_OVERFLOW);
      });
      
});

Tools Used

Hardhat

Recommended Mitigation Steps

Devs are using uint112 to store the claim amounts thinking this is enough, and this certainly is. But to overcome the overflow caused by the multiplication in line 176, one solution would be casting. The line could be changed like this:

uint112 linearVestAmount = uint112(uint152(_claim.linearVestAmount) * truncatedCurrentVestingDurationSecs / finalVestingDurationSecs);

This way the multiplication would fit in a uint152 and after division by finalVestingDurationSecs we would downcast back to uint112. Note here that finalVestingDurationSecs is always greater than or equal totruncatedCurrentVestingDurationSecs, so the downcast will not give us any trouble.

The text was updated successfully, but these errors were encountered:

All reactions