Solana Programs - Huma Finance

In the Huma Finance Permissionless Program, lending pools support multiple modes, allowing lenders to participate under different configurations. Each mode is represented by a ModeConfig account, which defines specific lending parameters, such as periodic yield rates.

The function get_mode_yield_bps extracts yield basis points (bps) from these ModeConfig accounts and assigns them to a vector, mode_yield_bps, which is later used in critical pool operations. However, the function does not check for duplicate ModeConfig accounts, which can lead to incorrect yield calculations.

programs/permissionless/src/pool/pool_utils.rs

pub(crate) fn get_mode_yield_bps(
    remaining_account_infos: &[AccountInfo],
    pool_state: &PoolState,
    pool_config_key: Pubkey,
    program_id: &Pubkey,
) -> Result<Vec<f64>> {
    let num_mode = pool_state.mode_config_keys.len();
    require!(
        remaining_account_infos.len() >= num_mode,
        Error::TooFewModes
    );

    let mode_config_account_infos = &remaining_account_infos[0..num_mode];
    let mut mode_yield_bps: Vec<f64> = vec![0.0; num_mode];

    for account_info in mode_config_account_infos.iter() {
        let (mode_config, mode_index) = preconditions::only_valid_mode_config(
            pool_state,
            pool_config_key,
            account_info,
            program_id,
        )?;
        mode_yield_bps[mode_index] = mode_config.periodic_apy_bps;
    }

    Ok(mode_yield_bps)
}

A duplicated ModeConfig results in one of the mode_yield_bps entries being 0.0, leading to incorrect asset and cumulative yield calculations in pre-closure mode.

When mode_yield_bps is passed to enter_pre_closure, the function refresh_assets_for_all_modes uses the incorrect values, preventing some assets from being refreshed.

programs/permissionless/src/pool/instructions/enter_pre_closure.rs

pub fn enter_pre_closure(&mut self, mode_yield_bps: &[f64]) -> Result<()> {
    self.refresh_assets_for_all_modes(mode_yield_bps)?;
    self.status = PoolStatus::PreClosure;

    Ok(())
}

Since mode_yield_bps is incorrect, any mode with a yield of 0.0 will not refresh its assets correctly in refresh_assets_for_all_modes.

programs/permissionless/src/pool/accounts.rs

fn refresh_assets_for_all_modes(&mut self, mode_yield_bps: &[f64]) -> Result<()> {
    assert!(!self.is_pool_closed());

    if !self.is_pool_in_pre_closure() {
        for (mode_state, yield_bps) in self
            .mode_states
            .iter_mut()
            .zip(mode_yield_bps.iter().cloned())
        {
            mode_state.refresh_assets(yield_bps)?;
        }
    }

    Ok(())
}

Finally, refresh_assets depends on a correct yield_bps value to update self.assets and self.cumulative_yields. Since a 0.0 yield prevents updates, cumulative_yields will remain incorrect permanently.

programs/permissionless/src/pool/accounts.rs

pub fn refresh_assets(&mut self, yield_bps: f64) -> Result<u128> {
    let current_ts = Clock::get()?.unix_timestamp as u64;
    let current_assets = self.assets;
    if self.assets_refreshed_at == 0 {
        self.assets_refreshed_at = current_ts;
        return Ok(current_assets);
    }

    let seconds_elapsed = current_ts - self.assets_refreshed_at;
    if seconds_elapsed > 0 {
        if yield_bps > 0.0 {
            let yield_rate_per_second =
                yield_bps / (SECONDS_IN_A_YEAR * HUNDRED_PERCENT_BPS as u64) as f64;
            self.assets = (current_assets as f64
                * (1_f64 + yield_rate_per_second).powi(seconds_elapsed as i32))
                as u128;
            self.cumulative_yields += self.assets - current_assets;
        }
        self.assets_refreshed_at += seconds_elapsed;
    }

    Ok(self.assets)
}

The risk of this issue is relatively low, as self.assets is updated in multiple parts of the code, ensuring that the total asset calculations remain mostly accurate.

However, self.cumulative_yields will not be updated once the pool enters pre-closure, leading to inconsistencies in yield tracking. Since self.cumulative_yields is used exclusively for calculating the cumulative yield of each mode, the affected mode’s cumulative yield will be incorrectly updated, potentially leading to discrepancies in reported earnings. While this does not immediately impact functionality, it could result in data inconsistency, especially in historical yield tracking and analytics.

To prevent duplicate ModeConfig accounts, update get_mode_yield_bps to enforce uniqueness before processing them.

SOLVED: The Huma Finance team solved the issue by modifying get_mode_yield_bps.

A boolean array has been introduced, initialized to false, to track processed modes. During iteration, the corresponding index is set to true for each mode. After the loop completes, a validation check ensures all expected entries are set, preventing duplicate modes and ensuring proper updates.

In the Huma Finance Permissionless program, change_pool_owner and set_pool_owner_treasury allow modifying critical ownership parameters of a pool. change_pool_owner updates the owner of a pool, while set_pool_owner_treasury updates the address of the treasury that manages the pool’s funds.

Ownership transitions should follow a two-step process: the current owner initiates the transfer, and the new owner explicitly accepts it. This prevents unintended transfers due to compromised keys or administrative errors.

However, as shown in the snippet below, both functions directly assign the new owner or treasury without requiring confirmation from the recipient.

permissionless/src/pool/instructions/update_pool_config.rs

pub(crate) fn change_pool_owner(ctx: Context<UpdatePoolConfig>, new_owner: Pubkey) -> Result<()> {
    let huma_config = preconditions::only_valid_huma_config(ctx.accounts.huma_config.as_ref())?;
    preconditions::only_pool_owner_or_huma_owner(
        &huma_config,
        ctx.accounts.pool_config.as_ref(),
        &ctx.accounts.signer,
    )?;

    let pool_state = ctx.accounts.pool_state.as_ref();
    pool_state.require_pool_not_closed()?;

    let pool_config = ctx.accounts.pool_config.as_mut();
    pool_config.pool_owner = new_owner; // Direct assignment without confirmation

    emit!(PoolOwnerChangedEvent {
        pool_id: pool_config.pool_id,
        owner: pool_config.pool_owner,
    });

    Ok(())
}

permissionless/src/pool/instructions/update_pool_config.rs

pub(crate) fn set_pool_owner_treasury(
    ctx: Context<UpdatePoolConfig>,
    new_treasury: Pubkey,
) -> Result<()> {
    let huma_config = preconditions::only_valid_huma_config(ctx.accounts.huma_config.as_ref())?;
    preconditions::only_pool_owner_or_huma_owner(
        &huma_config,
        ctx.accounts.pool_config.as_ref(),
        &ctx.accounts.signer,
    )?;

    let pool_state = ctx.accounts.pool_state.as_ref();
    pool_state.require_pool_not_closed()?;

    let pool_config = ctx.accounts.pool_config.as_mut();
    pool_config.pool_owner_treasury = new_treasury; // Direct assignment without confirmation

    emit!(PoolOwnerTreasuryChangedEvent {
        pool_id: pool_config.pool_id,
        old_treasury: ctx.accounts.pool_config.pool_owner_treasury,
        new_treasury,
    });

    Ok(())
}

The lack of a two-step ownership transfer mechanism poses a security risk, as the pool ownership or treasury can be reassigned without the new owner’s explicit approval. If an incorrect or malicious address is provided, recovery would be impossible, leading to governance and financial risks.

While only the current pool owner or Huma owner can make these changes, enforcing explicit acceptance by the new owner or treasury manager would mitigate the risk of mistaken or unauthorized transfers.

Implement a two-step ownership transfer process by introducing a pending owner field that requires confirmation from the new owner before finalizing the transfer.

programs/permissionless/src/pool/instructions/update_pool_config.rs

pub(crate) fn initiate_pool_owner_transfer(
    ctx: Context<UpdatePoolConfig>,
    pending_owner: Pubkey,
) -> Result<()> {
    let huma_config = preconditions::only_valid_huma_config(ctx.accounts.huma_config.as_ref())?;
    preconditions::only_pool_owner_or_huma_owner(
        &huma_config,
        ctx.accounts.pool_config.as_ref(),
        &ctx.accounts.signer,
    )?;

    let pool_state = ctx.accounts.pool_state.as_ref();
    pool_state.require_pool_not_closed()?;

    let pool_config = ctx.accounts.pool_config.as_mut();
    pool_config.pending_owner = Some(pending_owner); // Store pending owner

    Ok(())
}

programs/permissionless/src/pool/instructions/update_pool_config.rs

pub(crate) fn accept_pool_owner_transfer(ctx: Context<UpdatePoolConfig>) -> Result<()> {
    let pool_config = ctx.accounts.pool_config.as_mut();
    
    require!(
        pool_config.pending_owner == Some(ctx.accounts.signer.key()),
        Error::UnauthorizedTransferAcceptance
    );

    pool_config.pool_owner = pool_config.pending_owner.unwrap();
    pool_config.pending_owner = None; // Clear pending owner

    emit!(PoolOwnerChangedEvent {
        pool_id: pool_config.pool_id,
        owner: pool_config.pool_owner,
    });

    Ok(())
}

By implementing a pending ownership field and requiring explicit confirmation, unauthorized or accidental transfers can be prevented, improving security and governance over the pool.

ACKNOWLEDGED: The Huma Finance team acknowledged this finding due to multisig controls for admin accounts will be employed —including the pool owner— and timelocks on all instructions that update the pool configuration will be implemented. This approach ensures that every change, including owner modifications, is reviewed by at least two people before execution, with additional scrutiny likely during the timelock period.

The Huma Finance team believes these measures are sufficient to prevent the accidental changes described in the issue, rendering a two-step implementation unnecessary.

In the Huma Finance Permissionless program, the add_mode function is responsible for adding a new mode to a pool. Modes represent different lending mechanisms that allow lenders to participate under varying configurations. When a new mode is added, its corresponding configuration is stored in mode_config_keys, and a new ModeState is appended to mode_states.

Currently, the function validates that the number of modes does not exceed the maximum allowed by checking if self.mode_config_keys.len() is strictly less than MAX_NUM_MODES.

As shown in the snippet below, the check uses < rather than <= to ensure that the incremented length does not exceed the limit:

programs/permissionless/src/pool/accounts.rs

    pub fn add_mode(&mut self, mode_config_key: Pubkey) -> Result<()> {
        require!(
            !self.mode_config_keys.contains(&mode_config_key),
            Error::DifferentModesRequired
        );
        require!(
            self.mode_config_keys.len() < MAX_NUM_MODES,
            Error::TooManyModes
        );

        self.mode_config_keys.push(mode_config_key);
        self.mode_states.push(ModeState::default());

        Ok(())
    }

Since the check is performed before updating the vector, a more readable approach would be to first append the mode and then validate the length, ensuring clarity in the logic.
This issue does not introduce a functional bug but affects code readability and maintainability. The current approach can lead to confusion for developers unfamiliar with the logic, increasing the risk of misinterpretation when making modifications. Improving readability by restructuring the validation would reduce potential misunderstandings and improve maintainability.

Rearrange the logic so that the mode is added first, and then the length validation occurs, improving the readability of the function.

programs/permissionless/src/pool/accounts.rs

    pub fn add_mode(&mut self, mode_config_key: Pubkey) -> Result<()> {
        require!(
            !self.mode_config_keys.contains(&mode_config_key),
            Error::DifferentModesRequired
        );

        self.mode_config_keys.push(mode_config_key);
        self.mode_states.push(ModeState::default());

        require!(
            self.mode_config_keys.len() <= MAX_NUM_MODES,
            Error::TooManyModes
        );

        Ok(())
    }

ACKNOWLEDGED: The Huma Finance team acknowledged this finding.