Prepared by:
HALBORN
Last Updated 04/24/2025
Date of Engagement: March 28th, 2025 - April 2nd, 2025
100% of all REPORTED Findings have been addressed
All findings
7
Critical
0
High
0
Medium
1
Low
1
Informational
5
Smitthi
engaged Halborn to conduct a security assessment on their Vesting program beginning on April 2nd, 2025 and ending on April 7th, 2024. The security assessment was scoped to the smart contracts provided in the GitHub repository smitthi_vesting_contract, commit hashes, and further details can be found in the Scope section of this report.
The Smithii team
is releasing their smithii_vesting_contract
Solana program. This program allows for the creation of vesting/locking schedules and claiming of SPL tokens based on time or Merkle proofs.
Halborn was provided 4 days for the engagement and assigned one full-time security engineer to review the security of the Solana Programs in scope. The engineer is a blockchain and smart contract security expert with advanced smart contract hacking skills, and deep knowledge of multiple blockchain protocols.
The purpose of the assessment is to:
Identify potential security vulnerabilities within the codebase.
Verify the correctness of the core token locking, vesting schedule calculations, and claiming logic.
Assess access control mechanisms ensuring only authorized parties can perform sensitive actions.
Evaluate the security of state management, including initialization, updates, and potential data inconsistencies.
Analyze the implementation and usage of Merkle proofs for claim verification.
Identify potential edge cases or logical flaws that could lead to unexpected behavior, denial of service, or irrecoverable fund lockups.
Assess adherence to Solana development best practices regarding security, resource management (rent and compute), and CPI handling.
In summary, Halborn
identified some improvements to reduce the likelihood and impact of multiple risks, which were mostly addressed by the Smithii team
. The main ones were the following:
When setting up a vesting schedule for multiple recipients using the advanced verification feature, require the creator to choose how the tokens will be split.
Do not allow setting up a new vesting schedule if the amount of tokens to be locked is zero, as this wastes fees.
Halborn performed a combination of manual review and security testing based on scripts to balance efficiency, timeliness, practicality, and accuracy in regard to the scope of this assessment. While manual testing is recommended to uncover flaws in logic, process, and implementation; automated testing techniques help enhance coverage of the code and can quickly identify items that do not follow the security best practices. The following phases and associated tools were used during the assessment:
Research into architecture and purpose.
Differences analysis using GitLens to have a proper view of the differences between the mentioned commits
Graphing out functionality and programs logic/connectivity/functions along with state changes
EXPLOITABILITY METRIC () | METRIC VALUE | NUMERICAL VALUE |
---|---|---|
Attack Origin (AO) | Arbitrary (AO:A) Specific (AO:S) | 1 0.2 |
Attack Cost (AC) | Low (AC:L) Medium (AC:M) High (AC:H) | 1 0.67 0.33 |
Attack Complexity (AX) | Low (AX:L) Medium (AX:M) High (AX:H) | 1 0.67 0.33 |
IMPACT METRIC () | METRIC VALUE | NUMERICAL VALUE |
---|---|---|
Confidentiality (C) | None (I:N) Low (I:L) Medium (I:M) High (I:H) Critical (I:C) | 0 0.25 0.5 0.75 1 |
Integrity (I) | None (I:N) Low (I:L) Medium (I:M) High (I:H) Critical (I:C) | 0 0.25 0.5 0.75 1 |
Availability (A) | None (A:N) Low (A:L) Medium (A:M) High (A:H) Critical (A:C) | 0 0.25 0.5 0.75 1 |
Deposit (D) | None (D:N) Low (D:L) Medium (D:M) High (D:H) Critical (D:C) | 0 0.25 0.5 0.75 1 |
Yield (Y) | None (Y:N) Low (Y:L) Medium (Y:M) High (Y:H) Critical (Y:C) | 0 0.25 0.5 0.75 1 |
SEVERITY COEFFICIENT () | COEFFICIENT VALUE | NUMERICAL VALUE |
---|---|---|
Reversibility () | None (R:N) Partial (R:P) Full (R:F) | 1 0.5 0.25 |
Scope () | Changed (S:C) Unchanged (S:U) | 1.25 1 |
Severity | Score Value Range |
---|---|
Critical | 9 - 10 |
High | 7 - 8.9 |
Medium | 4.5 - 6.9 |
Low | 2 - 4.4 |
Informational | 0 - 1.9 |
Critical
0
High
0
Medium
1
Low
1
Informational
5
Security analysis | Risk level | Remediation Date |
---|---|---|
Potential Denial of Service for Claimants Due to Inconsistent Merkle Distribution Logic | Medium | Solved - 04/17/2025 |
Lack of Zero Amount Check in Initialization Allows Creation of Useless Vesting Schedules | Low | Solved - 04/17/2025 |
Precision Loss in Periodic Vesting Calculation Leaves Residual Funds | Informational | Partially Solved - 04/17/2025 |
Initialization Does Not Validate End Dates Are in the Future | Informational | Solved - 04/17/2025 |
Initialization Ignores Merkle Root if receiver_count is Zero | Informational | Solved - 04/22/2025 |
Redundant Check if fee != 0 in Initialization Functions | Informational | Solved - 04/17/2025 |
Use of unwrap on Optional Parameters May Cause Panics in Initialization | Informational | Solved - 04/17/2025 |
//
The Smithii Token Vesting program allows authorities to lock SPL tokens and release them based on time schedules, optionally distributing them to a list of receivers verified via Merkle proofs using the init_vesting_v2
and claim_vesting_v2
functions. A core issue arises because the program cannot determine the intended nature of the distribution encoded in the merkle_root
(whether it represents an equal average distribution, specific amounts per user, or a mix of both) solely from the total_amount
, receiver_count
, and merkle_root
provided during initialization. The program does not validate the internal consistency between these parameters and the actual distribution scheme encoded within the off-chain generated merkle_root
, allowing potentially conflicting setups.
Consider an example:
total_amount = 180
tokens, receiver_count = 10
.
The off-chain list used for the merkle_root
intends for 9 users to receive 10 tokens each, and 1 user to receive 90 tokens (sum = 180).
However, the on-chain logic in claim_vesting_v2
will calculate the average share as 180 / 10 = 18
tokens for anyone claiming without a specific claim_amount
.
If the user entitled to 90 tokens claims first using claim_amount = Some(90)
, only 90 tokens remain.
When the other 9 users attempt to claim their share using claim_amount = None
, the program will try to give them 18 tokens each based on the initial calculation, leading to insufficient funds after the first 5 of these users claim (5 * 18 = 90).
The snippet below shows the current state of the function:
programs/token-vesting/src/instructions/claim_vesting_v2.rs
let authority_key = ctx.accounts.authority.key();
let leaf_input = match claim_amount {
Some(amt) => format!("{}{}", authority_key.to_string(), amt),
None => authority_key.to_string(),
};
let current_timestamp = clock::Clock::get()?.unix_timestamp as u64;
let mut vested_amount: u64 = vesting.total_amount;
if vesting.receiver_count > 0 {
// ... Merkle proof verification using leaf_input ...
// Calculate amount per user if claim_amount is None
let amt_per_user = vesting.total_amount / (vesting.receiver_count as u64);
// Determine base vested amount based on presence of claim_amount input
vested_amount = claim_amount.unwrap_or(amt_per_user);
} else {
// ... logic for authority claim ...
}
// ... adjust vested_amount based on periods ...
This mismatch occurs because the claim_vesting_v2
function determines the amount to attempt claiming based on the claim_amount: Option<u64>
parameter provided by the caller, deciding the leaf format for verification and the base amount calculation. If claim_amount
is Some(amt)
, it uses amt
as the base amount and expects a proof for hash(address + amt)
. If claim_amount
is None
, it calculates an average share (amt_per_user = vesting.total_amount / vesting.receiver_count
) and expects a proof for hash(address)
. This distinction in claim logic, combined with the inability to determine the tree's intended nature at initialization, enables the potential DoS scenario.
If a vesting schedule is initialized with parameters inconsistent with a mixed-allocation Merkle root (as illustrated in the Description's example), a Denial of Service (DoS) condition can arise. Users intended to receive specific amounts (potentially larger than the calculated average total_amount / receiver_count
) can claim first by providing claim_amount = Some(their_specific_amount)
and a valid proof for hash(address + amount)
. Their successful claims can deplete the funds in the vesting_ata
disproportionately. Consequently, subsequent users who were intended to receive the calculated average share, and who call claim_vesting_v2
with claim_amount = None
and a valid proof for hash(address)
, may find insufficient funds remaining in the vesting_ata
. Their claim transactions will fail when the underlying SPL token transfer fails, effectively denying them service and preventing them from claiming their rightful share of the vested tokens.
Define Allocation & Tree:
Plan a mixed allocation (e.g., total = 180
, count = 10
; intent: 1 user gets 90 tokens, 9 users get 10 tokens each). Note the calculated on-chain average is 180 / 10 = 18
.
Generate leaf data: For the specific user, use user_pubkey_string + specific_amount_string
(e.g., "PublicKeyUser90" + "90"
). For average users, use user_pubkey_string
(e.g., "PublicKeyUser10_A"
).
Hash each leaf data entry using Keccak256.
Construct a Merkle tree from these hashed leaves and get the merkle_root
.
Initialize: Call init_vesting_v2
with total_amount = 180
, receiver_count = 10
, the generated merkle_root
, designation
, and a locking_end_date
set in the past (empty periods
).
Claim Specific (Success): Have the user intended for 90 tokens call claim_vesting_v2
with claim_amount = Some(90)
and their valid proof for their specific leaf (hash(PublicKeyUser90 + "90")
). Verify the transaction succeeds and transfers 90 tokens, leaving 90 in the vesting account.
Claim Average (Success x5): Have the first five users intended for 10 tokens call claim_vesting_v2
with claim_amount = None
and their valid proof for their address leaf (hash(PublicKeyUser10_A)
). Verify each transaction succeeds and transfers the calculated average of 18 tokens. The vesting account balance reaches 0 after the fifth claim.
Attempt Claim Average (Failure - DoS): Have the sixth user intended for 10 tokens call claim_vesting_v2
with claim_amount = None
and their valid proof.
Observe Failure: Confirm this transaction fails due to insufficient funds when the program attempts to transfer the calculated 18 tokens from the now-empty vesting account.
Introduce a mechanism to explicitly define and enforce the intended distribution type (Average, Specific, or Mixed) for Merkle tree-based vesting schedules created via init_vesting_v2
. This prevents the ambiguity that leads to the potential DoS scenario by ensuring the claim logic matches the structure of the provided Merkle root:
Modify Vesting
State: Add fields to the Vesting
struct (programs/token-vesting/src/state/vesting.rs
) to store the distribution type and, conditionally, the default amount for mixed distributions. Define an enum for clarity. Update the SPACE
constant.
programs/token-vesting/src/state/vesting.rs
use anchor_lang::prelude::*;
// Define enum for distribution type
#[derive(AnchorSerialize, AnchorDeserialize, Debug, Clone, PartialEq, Eq, Copy)]
pub enum DistributionType {
Average, // All receivers get total_amount / receiver_count
Specific, // All receivers claim with specific amount in proof
Mixed // Some specific, some default amount
}
#[account]
pub struct Vesting {
// ... existing fields: authority, token_mint, total_amount, etc. ...
pub receiver_count: u16,
pub merkle_root: [u8; 32],
// --- New Fields ---
pub distribution_type: DistributionType, // Type of distribution for merkle root
pub default_claim_amount: Option<u64>, // Required only if type is Mixed
pub bump: u8,
// ... existing padding ...
}
// Remember to update Vesting::SPACE constant to account for new fields
// Size of enum (usually 1 byte) + Option<u64> (1 byte discriminator + 8 bytes payload)
Update init_vesting_v2
Instruction:
Add distribution_type: DistributionType
and default_claim_amount: Option<u64>
as parameters to the init_v2
instruction method.
Implement validation:
If distribution_type
is Mixed
, default_claim_amount
must be Some(amount)
where amount > 0
.
If distribution_type
is Average
or Specific
, default_claim_amount
must be None
.
Store the validated distribution_type
and default_claim_amount
in the Vesting
account state during initialization.
programs/token-vesting/src/instructions/init_vesting_v2.rs
// Add new arguments to init_vesting_v2 function signature:
pub fn init_vesting_v2(
ctx: Context<InitVestingV2>,
designation: u8,
// ... existing args: periods_count, periods, total_amount, receiver_count, locking_end_date, merkle_root ...
distribution_type: DistributionType, // New argument
default_claim_amount: Option<u64> // New argument
) -> Result<()> {
// ... existing initial setup ...
// --- New Validation Logic ---
match distribution_type {
DistributionType::Mixed => {
require!(default_claim_amount.is_some(), ErrorCode::MissingDefaultAmountForMixed); // New error
require_gt!(default_claim_amount.unwrap(), 0, ErrorCode::ZeroDefaultAmountForMixed); // New error
},
DistributionType::Average | DistributionType::Specific => {
require!(default_claim_amount.is_none(), ErrorCode::UnexpectedDefaultAmount); // New error
}
}
// --- End New Validation ---
// ... existing logic (token transfer, fee payment) ...
// In vesting.initialize() call (or directly before):
vesting.distribution_type = distribution_type;
vesting.default_claim_amount = default_claim_amount;
// ... rest of vesting.initialize() ...
Ok(())
}
Update claim_vesting_v2
Instruction: Modify the claim logic to read distribution_type
and act accordingly:
programs/token-vesting/src/instructions/claim_vesting_v2.rs
let vesting = &mut ctx.accounts.vesting;
let claim_account = &mut ctx.accounts.claim_account;
let authority_key = ctx.accounts.authority.key();
// provided_claim_amount is the Option<u64> argument passed to the instruction
let mut leaf_input: String;
let mut base_amount_for_periods: u64;
// Determine expected leaf format and base amount based on stored type
match vesting.distribution_type {
DistributionType::Average => {
require!(provided_claim_amount.is_none(), ErrorCode::UnexpectedClaimAmount); // New error
leaf_input = authority_key.to_string();
base_amount_for_periods = vesting.total_amount / (vesting.receiver_count as u64);
},
DistributionType::Specific => {
require!(provided_claim_amount.is_some(), ErrorCode::MissingClaimAmount); // New error
let amount = provided_claim_amount.unwrap();
require_gt!(amount, 0, ErrorCode::ZeroClaimAmount); // New error
leaf_input = format!("{}{}", authority_key.to_string(), amount);
base_amount_for_periods = amount;
},
DistributionType::Mixed => {
match provided_claim_amount {
Some(amount) => { // User claims a specific amount
require_gt!(amount, 0, ErrorCode::ZeroClaimAmount);
leaf_input = format!("{}{}", authority_key.to_string(), amount);
base_amount_for_periods = amount;
},
None => { // User claims the default/average amount for this mixed setup
leaf_input = authority_key.to_string();
// Use the stored default amount, ensure it exists (checked at init)
base_amount_for_periods = vesting.default_claim_amount.ok_or(ErrorCode::MissingDefaultAmountForMixed)?; // Should not fail if init validation is correct
}
}
}
}
Important note on total_amount
: It is crucial to understand that this recommended on-chain solution still relies fundamentally on the authority initializing the vesting (init_vesting_v2
) to perform correct off-chain calculations. The authority must ensure that the total_amount
deposited into the contract precisely matches the sum of all specific token amounts or the calculated total for an average distribution (or the sum of specific + default amounts for mixed distributions), corresponding exactly to the allocations encoded within the provided Merkle root and declared distribution_type
. The on-chain program enforces consistency in claim processing based on the declared type but cannot validate the deposited total_amount
against the sum implied by the Merkle tree's contents.
SOLVED:
The Smithii team
solved the issue by modifying implementing the suggested changes.
//
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:
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.
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:
Get Current Timestamp: Obtain an estimate of the current Unix timestamp (current_timestamp
).
Define Past Date: Choose a Unix timestamp (past_end_date
) such that past_end_date <= current_timestamp
.
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.
Execute init
: Call the initialization instruction with these parameters.
Observe Success: Note that the initialization transaction completes successfully without errors.
Fetch State: Read the data from the Vesting
state account created by the transaction.
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:
Prepare Input: Generate a non-zero 32-byte array to serve as a dummy Merkle root (dummy_merkle_root
).
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)
.
Observe Success: Note that the initialization transaction completes successfully without errors.
Fetch State: Read the data stored in the Vesting
state account created by the transaction.
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:
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;
}
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.
Halborn used automated security scanners to assist with the detection of well-known security issues and vulnerabilities. Among the tools used was cargo-audit
, a security scanner for vulnerabilities reported to the RustSec Advisory Database. All vulnerabilities published in https://crates.io are stored in a repository named The RustSec Advisory Database. cargo audit
is a human-readable version of the advisory database which performs a scanning on Cargo.lock. Security Detections are only in scope. All vulnerabilities shown here were already disclosed in the above report. However, to better assist the developers maintaining this code, the reviewers are including the output with the dependencies tree, and this is included in the cargo audit
output to better know the dependencies affected by unmaintained and vulnerable crates.
ID | package | Short Description |
---|---|---|
RUSTSEC-2022-0093 | ed25519-dalek | Double Public Key Signing Function Oracle Attack on |
Halborn strongly recommends conducting a follow-up assessment of the project either within six months or immediately following any material changes to the codebase, whichever comes first. This approach is crucial for maintaining the project’s integrity and addressing potential vulnerabilities introduced by code modifications.
// Download the full report
Vesting
* Use Google Chrome for best results
** Check "Background Graphics" in the print settings if needed