Description

The deserialize_allocation() function performs BCS deserialization of allocation data without validating the integrity of the deserialized values. This creates multiple denial-of-service vectors that will manifest at withdrawal time rather than at merkle root creation time.

Missing validations:

1. Calendar schedule: No check that unlock_timestamps and unlock_amounts arrays have equal length

2. Interval schedule: No check that period_length > 0 or number_of_periods > 0

3. Amount consistency: No check that allocation.amount matches sum of unlock amounts or piece amounts

4. Array bounds: No maximum size limits on timestamps/amounts/pieces vectors

#1 - Array length mismatch:

In deserialize_allocation():

Schedule::Calendar {
    unlock_timestamps: stream.peel_vec!(|stream| stream.peel_u32()),
    unlock_amounts: stream.peel_vec!(|stream| stream.peel_u64()),
}
// No validation that lengths match

Later in calc_vested_amount_calendar():

let timestamps_length = unlock_timestamps.length();
while (i < timestamps_length) {
    if (unlock_timestamps[i] <= final_timestamp) {
        amount = amount + unlock_amounts[i]; // ← Runtime panic if unlock_amounts.length() < timestamps_length
    };
    i = i + 1;
};

#2 Division by zero:

In deserialize_allocation():

Piece {
    start_time: stream.peel_u32(),
    period_length: stream.peel_u32(),  // ← No check that this > 0
    number_of_periods: stream.peel_u32(),  // ← No check that this > 0
    amount: stream.peel_u64(),
}

Later in calc_vested_piece_amount():

let mut fully_vested_periods = elapsed_time / piece.period_length;  // ← Division by zero panic

// ...later
piece.amount * (fully_vested_periods as u64) / (piece.number_of_periods as u64)  // ← Division by zero panic

#3 - No array size caps:

let pieces = stream.peel_vec!(|stream| { /* ... */ });
// No check on pieces.length() - could be 10,000+ entries

Used in gas-intensive loops:

fun calc_vested_amount_interval(final_timestamp: u32, pieces: &vector<Piece>): u64 {
    let pieces_length = pieces.length();  // Could be unbounded
    let mut amount = 0;
    let mut i = 0;
    while (i < pieces_length) {  // ← Gas exhaustion with large arrays
        amount = amount + calc_vested_piece_amount(&pieces[i], final_timestamp);
        i = i + 1;
    };
    amount
}

Impact

Permanent withdrawal DoS:

• Mismatched array lengths cause runtime panics in calc_vested_amount_calendar() during withdrawal

• Zero values in period_length or number_of_periods cause division-by-zero panics during withdrawal

• Once a merkle root containing malformed data is added, affected allocations become permanently unclaimable

Fund locking:

• If sum(unlock_amounts) < allocation.amount, excess funds remain permanently locked in the contract

• If sum(piece.amount) < allocation.amount, beneficiaries cannot claim their full allocation

Gas exhaustion:

• Unbounded array sizes (e.g., 10,000 unlock timestamps) can make withdrawal transactions consistently fail due to gas limits, effectively locking funds

Operational risk:

• These issues stem from off-chain tooling errors: CSV export bugs, spreadsheet formula errors, script typos, or copy-paste mistakes

• Without validation, innocent operational mistakes permanently brick user allocations

• Discovery happens at withdrawal time (potentially months/years after merkle root creation), making remediation impossible since merkle roots cannot be removed

Recommendations

Add comprehensive validation checks in deserialize_allocation():

1. Array length validation: Assert that calendar unlock_timestamps and unlock_amounts have equal length

2. Zero-value guards: Assert that period_length > 0 and number_of_periods > 0 for all interval pieces

3. Array size caps: Limit maximum entries to reasonable bounds to prevent gas exhaustion

4. Non-empty validation: Assert that calendar schedules have at least one unlock and interval schedules have at least one piece

5. Amount validation: Optionally verify that sum of unlock_amounts or piece amounts equals the total allocation amount

These validations should occur during deserialization to fail-fast at withdrawal time, or ideally add a view function to validate allocations before calling add_merkle_root().

Remediation

Magna provided the following response to this finding:

This is a conscious design decision, we wouldn’t like to waste gas by doing those checks on-chain, those checks are done offchain when building the tree. All checks are implicitly ‘done’ on-chain by verifying the merkle proof against the root. The root is assumed to be correct.