Vesting - Smithii

The init_vesting and init_vesting_v2 functions are responsible for creating new vesting schedules. They accept a total_amount parameter specifying the number of tokens to be vested. However, neither function validates whether this total_amount is greater than zero before proceeding with the initialization logic, including transferring tokens (which succeeds for a zero amount) and initializing the Vesting state account, as demonstrated conceptually in the code snippet below (example from init_vesting). This allows for the creation of vesting schedules intended to lock zero tokens.

pub fn init_vesting(
  ctx: Context<InitVesting>,
  periods_count: u8,
  periods: Vec<VestingPeriod>,
  total_amount: u64, // Accepts any u64, including 0
  receiver_count: u16,
  locking_end_date: Option<u64>,
  merkle_root: Option<[u8; 32]>
) -> Result<()> {
  msg!("Logging with a variable: {}", periods_count);
  let authority = &ctx.accounts.authority;
  let authority_ata = &mut ctx.accounts.authority_ata;
  let vesting = &mut ctx.accounts.vesting;
  let vesting_ata = &mut ctx.accounts.vesting_ata;
  let token_mint = &ctx.accounts.token_mint;
  let authority_pubkey: Pubkey = authority.key();
  let bump = ctx.bumps.vesting;

  // No check here to ensure total_amount > 0

  if token_mint.freeze_authority.is_some() {
    return err!(ErrorCode::TokenFreezeAuthority);
  }

  transfer( // Transfer will proceed even if total_amount is 0
    CpiContext::new(ctx.accounts.token_program.to_account_info(), Transfer {
      from: authority_ata.to_account_info(),
      to: vesting_ata.to_account_info(),
      authority: authority.to_account_info(),
    }),
    total_amount // Uses potentially zero amount
  )?;
  // ... rest of initialization proceeds

Allowing total_amount to be zero leads to the successful creation of functionally useless vesting schedules. The primary impact is a waste of resources for the caller (authority): they pay the non-refundable SOL fee (0.1-0.3 SOL) to the program's vault and also pay SOL for the rent-exempt reserve of the created Vesting state account and its associated token account (vesting_ata), all for a schedule that locks no tokens. Any subsequent attempts to claim from such a schedule will fail (likely with NoClaimableToken), preventing withdrawal but confirming the schedule's uselessness. This could also lead to minor state clutter on-chain or user confusion if these zero-amount schedules appear in user interfaces.

Add input validation at the beginning of both the init_vesting and init_vesting_v2 functions to ensure the provided total_amount is strictly greater than zero. This can be achieved using the require_gt! macro from Anchor. A new error code should be added to errors.rs to handle this specific case.

SOLVED: The Smithii team solved the issue by checking that the total_amount parameter is bigger than 0.

The smithii_vesting_contract program enables an authority to lock SPL tokens and distribute them over time to beneficiaries according to a schedule defined by periods. The claim_vesting and claim_vesting_v2 functions calculate and transfer the token amount a user can claim based on elapsed periods. It has been identified that the formula used within both claim_vesting and claim_vesting_v2 for calculating the token amount per claim when using periods introduces rounding errors due to integer division, as demonstrated in the code snippet below (example from claim_vesting.rs).

programs/token-vesting/src/instructions/claim_vesting.rs

} else {
    let mut total_percent = 0;

    for period in vesting.periods.iter() {
      if current_timestamp >= period.end_date {
        total_percent += period.percentage;
      } else {
        break;
      }
    }

    let percent = (total_percent as i64) - (claim_account.claimed_percent as i64);

    require_gt!(percent, 0, ErrorCode::NoClaimableToken);

    amount = (amount / 100) * (percent as u64);

    claim_account.claimed_percent = total_percent;
  }

The calculation amount = (amount / 100) * (percent as u64); performs the division before the multiplication, which can truncate residuals. While reversing the order helps ((amount * percent) / 100), a more robust solution avoids repeated percentage calculations on the base amount, instead focusing on the total amount vested versus the total amount already claimed.

The integer division flaw present in both claim functions means that during each partial claim before the final period, a slightly smaller amount than theoretically due for that percentage might be delivered, regardless of which claim function (claim_vesting or claim_vesting_v2) is used. When the final period is reached and its portion is calculated using the same potentially truncating formula, the total sum of amounts delivered across all claims may be less than the user's initial total allocation. This results in a small amount of tokens becoming permanently locked in the contract's vesting account. Consequently, users do not receive the full amount of tokens allocated to them, representing a loss of funds for the end beneficiary, even after 100% of the vesting period should have elapsed.

Consider implementing the next two recommendations:

1. Modify the Claim account state (programs/token-vesting/src/state/claim.rs) to include a new field, claimed_amount: u64, alongside the existing claimed_percent: u8. This new field will track the cumulative absolute amount withdrawn and is essential for calculating the exact remaining balance to be delivered during the final claim period (total_percent == 100). Remember to update the account's SPACE constant accordingly.

   
   
   

2. Update the calculation logic within both the claim_vesting and claim_vesting_v2 instructions. First, calculate the percentage points to claim in the current transaction (let percent = (total_percent as i64) - (claim_account.claimed_percent as i64);). Then, differentiate the logic:

   • If it is the final claim (total_percent == 100), calculate the amount_to_claim_now using the newly added claimed_amount field and the user's base allocation (amount): amount.saturating_sub(claim_account.claimed_amount);.

   • If it is an intermediate claim (total_percent < 100), calculate the amount_to_claim_now based on the calculated percent for this transaction and the user's base allocation (amount), using the corrected formula to avoid precision loss and potential overflows: ((amount as u128 * percent as u128) / 100) as u64;.

   • Update both relevant state fields in the Claim account: claim_account.claimed_amount and claim_account.claimed_percent.

PARTIALLY SOLVED: The Smithii team partially solved the issue by updating only the claim_vesting_v2 instruction. Modifying claim_vesting was avoided because it would require account updates that would break backward compatibility with old-type accounts.

The Vesting::initialize method, called internally by init_vesting and init_vesting_v2, is responsible for populating the Vesting state account with all parameters for a new vesting schedule, including token details, total amount, and the release timeline defined by end dates. While this method correctly fetches the current blockchain timestamp via clock::Clock::get()?.unix_timestamp to set the locking_start_date, it fails to validate that user-provided end dates – either the single locking_end_date (if periods_count == 0) or the individual period.end_date values within the periods vector – are strictly greater than the locking_start_date (current time), as shown conceptually in the snippet below. The code assigns locking_end_date directly and iterates through periods checking only for sequential order (period.end_date <= last_end_date) but not ensuring each date is in the future relative to the initialization time. This allows creating vesting schedules with end dates set in the past or present.

programs/token-vesting/src/state/vesting.rs

self.locking_start_date = clock::Clock::get()?.unix_timestamp as u64;

      if periods_count == 0 {
        self.locking_end_date = locking_end_date.unwrap();
      } else {
        if periods_count > 5 || periods_count == 0 {
          return err!(ErrorCode::InvalidPeriods);
        }

        let mut total_percent = 0;
        let mut last_end_date = 0;

        for period in periods.iter() {
          total_percent += period.percentage;
          if period.end_date <= last_end_date {
            return err!(ErrorCode::InvalidEndDate);
          }
          last_end_date = period.end_date;
        }

        require_eq!(total_percent, 100, ErrorCode::InvalidPercentage);

        self.periods = periods.clone();
        self.locking_end_date = last_end_date;
      }

The lack of validation allows the creation of vesting schedules where tokens might be partially or fully claimable immediately upon initialization if past or present dates are provided for locking_end_date or period.end_date. This fundamentally undermines the purpose of time-based vesting or locking, as the intended delay mechanism can be bypassed entirely. It could be exploited by providing past dates to make tokens instantly available, or simply lead to misconfigured schedules due to user error, where the lock-up period does not behave as expected.

Steps to Reproduce:

1. Get Current Timestamp: Obtain an estimate of the current Unix timestamp (current_timestamp).

   
   

2. Define Past Date: Choose a Unix timestamp (past_end_date) such that past_end_date <= current_timestamp.

   
   

3. Prepare Parameters: Construct the parameters for init_vesting or init_vesting_v2:

   • Set total_amount > 0.

   • Either set periods to be non-empty, including at least one VestingPeriod where end_date is past_end_date.

   • Or set periods to empty (periods_count = 0) and set locking_end_date to Some(past_end_date).

   • Ensure other parameters are valid.

   
   

4. Execute init: Call the initialization instruction with these parameters.

   
   

5. Observe Success: Note that the initialization transaction completes successfully without errors.

   
   

6. Fetch State: Read the data from the Vesting state account created by the transaction.

   
   

7. Verify State: Confirm that the locking_end_date or the relevant period.end_date stored in the state matches the past_end_date provided, and that this date is less than or equal to the locking_start_date also stored in the state.

Implement validation checks within Vesting::initialize after obtaining the current time (locking_start_date) to ensure all provided end dates are strictly in the future.

SOLVED: The Smithii team solved the issue by checking that the dates are future dates.

The Vesting::initialize method, called internally by both init_vesting and init_vesting_v2, sets up the state for a new vesting schedule. It includes logic to handle an optional merkle_root and a receiver_count. However, the code only processes and stores the provided merkle_root if the receiver_count parameter is greater than zero, as shown in the snippet below. If a caller provides a valid merkle_root (i.e., Some(...)) but sets receiver_count to 0, the conditional block is skipped. Consequently, the provided Merkle root is ignored, and the Vesting state account is initialized with receiver_count = 0 and the default zero-array value for merkle_root, contradicting the likely intent of the caller who supplied a specific root.

programs/token-vesting/src/state/vesting.rs

if receiver_count > 0 {
      self.merkle_root = merkle_root.unwrap();
      if let Some(root) = merkle_root {
        self.merkle_root = root;
        self.receiver_count = receiver_count;
      } else {
        return err!(ErrorCode::NoMerkleRoot);
      }
    }

The primary risk is unexpected behavior and a potential logic flaw. When a user provides a Merkle root, the clear intention is to enable Merkle-proof-based claiming for a specific list of recipients. By ignoring the root when receiver_count is 0, the program silently creates a vesting schedule where only the original authority can claim tokens, completely bypassing the intended Merkle validation. This can lead to misconfigured vesting schedules that do not function as the creator intended, potentially requiring redeployment and incurring wasted rent, and fee costs from the initial incorrect setup. It does not directly lead to loss or theft of the vested tokens themselves but undermines the configuration's integrity.

Steps to reproduce:

1. Prepare Input: Generate a non-zero 32-byte array to serve as a dummy Merkle root (dummy_merkle_root).

   
   

2. Execute init: Call init_vesting or init_vesting_v2 with parameters where receiver_count is explicitly set to 0 and merkle_root is set to Some(dummy_merkle_root).

   
   

3. Observe Success: Note that the initialization transaction completes successfully without errors.

   
   

4. Fetch State: Read the data stored in the Vesting state account created by the transaction.

   
   

5. Verify State: Check the fetched state data and confirm that:

   • The receiver_count field is 0.

   • The merkle_root field is [0u8; 32] (all zeros), not the dummy_merkle_root provided as input.

To ensure consistency between the merkle_root and receiver_count parameters in the Vesting::initialize function, implement the following steps:

1. Validation Logic:

   • Add a validation block to ensure:

     • If merkle_root is provided (merkle_root.is_some()), receiver_count must be greater than zero.

     • If receiver_count is greater than zero, merkle_root must be provided.

     if merkle_root.is_some() && receiver_count == 0 {
         return err!(ErrorCode::InvalidReceiverCount);
     }
     
     if receiver_count > 0 && merkle_root.is_none() {
         return err!(ErrorCode::NoMerkleProof);
     }
     
     if receiver_count > 0 {
         self.merkle_root = merkle_root.unwrap();
         self.receiver_count = receiver_count;
     }

2. Error Handling:

   • Define new error codes in the ErrorCode enum for cases where:

     • merkle_root is missing but receiver_count is greater than zero.

     • receiver_count is zero but merkle_root is provided.

SOLVED: The Smithii team solved the issue by including functionality to check a correct combination of merkle root and receivers number.

Both the init_vesting and init_vesting_v2 functions calculate a SOL fee that the caller must pay. The logic initializes a mutable variable fee to a non-zero value (100_000_000 lamports) and potentially increases it based on whether vesting periods or receiver lists are used. Subsequently, before executing the system instruction to transfer this calculated fee to the vault, the code performs a check if fee != 0, as demonstrated in the snippet below (example from init_vesting.rs). Given that fee is initialized to 100_000_000 and can only be increased, it is guaranteed to be non-zero at the point of the check. Therefore, the condition fee != 0 will always evaluate to true, making the if statement redundant.

programs/token-vesting/src/instructions/init_vesting.rs

let mut fee: u64 = 100_000_000; // Initialized to non-zero value

    if periods_count > 0 {
        fee += 100_000_000;
    }

    if receiver_count > 0 {
        fee += 100_000_000;
    }

    // Check below is always true because fee >= 100_000_000
    if fee != 0 {
        let vault = &ctx.accounts.vault;
        // ... fee transfer logic ...
    }

This redundant check does not introduce a functional bug or security vulnerability, as the fee transfer logic within the if block executes correctly under the current implementation (since the condition is always met). However, the unnecessary if statement adds slight clutter to the codebase, potentially impacting readability and maintainability. It represents dead code in the sense that the conditionality it implies is never actually utilized. There might also be a negligible overhead in compute units consumed for evaluating an always-true condition. The primary risk is related to code quality rather than operational correctness.

For improved code clarity, quality, and potentially minor optimization, it is recommended to remove the redundant if fee != 0 check and its associated braces ({}) in both the init_vesting and init_vesting_v2 functions. The logic responsible for transferring the fee should be executed unconditionally following the fee calculation, as it is intended to run in all valid initialization scenarios under the current fee structure.

SOLVED: The Smithii team solved the issue by removing the redundant validations.

The Vesting::initialize method within programs/token-vesting/src/state/vesting.rs handles optional input parameters locking_end_date (type Option<u64>) and merkle_root (type Option<[u8; 32]>). However, under certain conditions, the code directly calls .unwrap() on these optional values instead of handling the None case gracefully, as shown in the snippets below. Specifically, locking_end_date.unwrap() is called if periods_count is 0, and merkle_root.unwrap() is called if receiver_count is greater than 0. In Rust and Anchor programs, calling .unwrap() on a None value triggers a program panic.

programs/token-vesting/src/state/vesting.rs

if periods_count == 0 {
      self.locking_end_date = locking_end_date.unwrap();
    } else {

if receiver_count > 0 {
      self.merkle_root = merkle_root.unwrap();
      if let Some(root) = merkle_root {

If the init_vesting or init_vesting_v2 instructions are invoked with parameters that lead to .unwrap() being called on a None value (e.g., setting periods_count = 0 but not providing locking_end_date, or setting receiver_count > 0 but not providing merkle_root), the program will panic. A panic results in an immediate and ungraceful transaction failure. The caller receives a generic runtime error message, making it difficult to diagnose the specific cause without inspecting detailed program logs.

Replace all calls to .unwrap() on Option types with explicit error handling that returns a Result::Err. Use methods like .ok_or() or .ok_or_else() combined with the ? operator or require! macros to check for None and return a specific, informative ErrorCode if a required optional parameter is missing.

SOLVED: The Smithii team solved the issue by implementing validations that the mentioned parameters are not None and throwing an error in case they are.