Blueprint: Glow Assessment - Blueprint Finance

The Margin Pool Adapter program allows an authority to create token pools using any Token2022 token and to transfer tokens into and out of the pool vault. However, the program does not validate whether the pool vault’s mint has any associated extensions, which may introduce security risks.

For example:

• If the mint includes the TransferFeeConfig extension, a fee is automatically deducted from each transfer. This means the amount received in the destination pool vault token account will be less than expected, potentially causing data inconsistencies in deposits and loan repayments. The recorded deposit/repayment amount may not match the actual amount transferred.

• If the mint includes the PermanentDelegate extension, a designated delegate has unrestricted permissions to transfer and burn tokens from any account associated with that mint, which could lead to unauthorized fund movements.

create_pool.rs

/// The mint for the token being custodied by the pool
pub token_mint: Box<InterfaceAccount<'info, Mint>>,

To address this issue, it is recommended to validate and restrict the use of mints with potentially harmful extensions.

PARTIALLY SOLVED: The Glow team partially resolved this issue by validating underlying mints during pool creation and rejecting those with potentially harmful extensions. However, to support the PYUSD (PayPal USD) token, which has the PermanentDelegate extension enabled, this extension needed to remain allowed. To mitigate potential risks, it is necessary to manually review all newly created pools and their underlying mints, ensuring that only mints with the PermanentDelegate extension from trusted third parties are accepted.

The set_entry instruction of the Metadata program and the create_lookup_table instruction of the Lookup Table Registry program both use the realloc function to expand the size of data accounts used by these programs and re-initialize the new memory to zero.

The realloc function includes a boolean parameter that determines whether the memory should be re-initialized to zero. Re-initializing memory is helpful for preventing the reuse of old data when the program first decreases the memory size and later increases it again. However, when the account size is only increased, re-initialization is unnecessary because the runtime already provides pre-allocated, zero-filled memory. Performing this re-initialization in such cases results in wasted compute units.

As both instructions can only increase the account data size, memory re-initialization is therefore not necessary.

metadata/src/lib.rs

pub fn set_entry(ctx: Context<SetEntry>, offset: u64, data: Vec<u8>) -> Result<()> {
    // Check if the metadata account needs to be resized
    let metadata_account = ctx.accounts.metadata_account.to_account_info();

    let offset: usize = offset as usize;
    let data_len = data.len() + offset;
    let account_len = metadata_account.data_len();
    if account_len < data_len {
        // We need to realloc
        let rent = Rent::get()?;
        let transfer_amount = rent
            .minimum_balance(data_len)
            .saturating_sub(metadata_account.lamports());

        if transfer_amount > 0 {
            anchor_lang::system_program::transfer(
                CpiContext::new(
                    ctx.accounts.system_program.to_account_info(),
                    anchor_lang::system_program::Transfer {
                        from: ctx.accounts.payer.to_account_info(),
                        to: metadata_account.clone(),
                    },
                ),
                transfer_amount,
            )?;
        }

        metadata_account.realloc(data_len, true)?;
    }

lookup-table-registry/src/lib.rs

if append_to_end {
    // Happy case, add to the end
    let registry_info = ctx.accounts.registry_account.to_account_info();
    let existing_len = registry_info.data_len();
    registry_info.realloc(existing_len + REGISTRY_ENTRY_SIZE, true)?;
    ctx.accounts.registry_account.tables.push(entry);
} else {

To address this issue it is recommended to disable the memory re-initialization feature of the realloc function when the account data size is only being increased (without first being decreased and then increased again).

SOLVED: The Glow team solved this finding by disabling the memory re-initialization.

The instructions set_governor and airspace_set_authority allow an authority to set a new authority of the protocol and the airspace, respectively.

However, both instructions pass the new authority is as Public key via instruction parameters and do not require the new authority to sign the instruction. Accidentally setting an incorrect authority would cause loss of control over the protocol or airspace.

set_governor.rs

pub fn set_governor_handler(ctx: Context<SetGovernor>, new_governor: Pubkey) -> Result<()> {
    ctx.accounts.governor_id.governor = new_governor;

    Ok(())
}

airspace_set_authority.rs

pub fn airspace_set_authority_handler(
    ctx: Context<AirspaceSetAuthority>,
    new_authority: Pubkey,
) -> Result<()> {
    let airspace = &mut ctx.accounts.airspace;

    airspace.authority = new_authority;

    emit!(AirspaceAuthoritySet {
        airspace: airspace.key(),
        authority: new_authority
    });

    Ok(())
}

To address this issue, it is recommended to require the new authority to sign the instruction as well, ensuring they possess the private key.

SOLVED: The Glow team resolved this finding by implementing a two-step process. First, an instruction is executed to propose a new authority. Then, a second instruction requires the proposed authority to finalize the transfer by providing its signature.

The configure_token instruction allows the airspace authority to set and update token configurations. However, the parameters value_modifier, max_staleness, admin and token_kind are not validated, leaving them open to being set to arbitrary values:

• max_staleness: If this parameter is incorrectly set to 0 or an excessively large value, it will effectively disable the program's staleness verification mechanism.

• value_modifier: If this parameter is set to a value greater than 100, it could enable the issuance of under-collateralized loans.

• token_kind: Accidentally updating the token kind from Claim to Collateral or vice versa would corrupt the data integrity of the protocol that might cause losses for the protocol or for the users.

• admin: Accidentally setting an incorrect account as token administrator could compromise the token price results. In addition the instruction refresh_deposit_position does not verify the correct owner of the Pyth oracle price account which could be an issue in case the oracle is set incorrectly during token configuration.

configure_token.rs

pub fn configure_token_handler(
    ctx: Context<ConfigureToken>,
    updated_config: Option<TokenConfigUpdate>,
) -> Result<()> {
    let config = &mut ctx.accounts.token_config;

    emit!(TokenConfigured {
        airspace: ctx.accounts.airspace.key(),
        mint: ctx.accounts.mint.key(),
        update: updated_config.clone(),
    });

    let updated_config = match updated_config {
        Some(update) => update,
        None => return config.close(ctx.accounts.payer.to_account_info()),
    };

    if config.underlying_mint != Pubkey::default()
        && updated_config.underlying_mint != config.underlying_mint
    {
        msg!("underlying mint cannot be changed");
        return err!(ErrorCode::InvalidConfig);
    }

    config.mint = ctx.accounts.mint.key();
    config.airspace = ctx.accounts.airspace.key();
    config.underlying_mint = updated_config.underlying_mint;
    config.admin = updated_config.admin;
    config.token_kind = updated_config.token_kind;
    config.value_modifier = updated_config.value_modifier;
    config.max_staleness = updated_config.max_staleness;

    config.validate()?;

    Ok(())
}

The configure margin pool instruction allows the airspace authority to set and update pool configurations. However non of the MarginPoolConfig parameters are validated, leaving them open to being set to arbitrary values.

The program does not guarantee, that the parameters utilization_rate_* and borrow_rate_* are set with increasing values which could cause unexpected behavior. The parameter management_fee_rate is not capped to 10000 and setting it to greater value could result in incorrect fee and interest calculations.

configure.rs

pub fn configure_handler(
    ctx: Context<Configure>,
    metadata: Option<TokenMetadataParams>,
    config: Option<MarginPoolConfig>,
    oracle: Option<TokenPriceOracle>,
) -> Result<()> {
    let pool = &mut ctx.accounts.margin_pool;

    if let Some(new_config) = config {
        pool.config = new_config;
    }

Proper validation of these parameters is essential to ensure the program functions as intended and avoids potential misuse.

To address this issue, it is recommended to validate the value_modifier and max_staleness parameters by restricting their values to predefined acceptable ranges. The token_kind parameter must be set only during the initial token configuration and cannot not be allowed to be modified afterward. When setting the admin parameter, the program must verify that the oracle price account has correct data format and is owned by the Pyth program.

Validate the MarginPoolConfig parameters of the configure instruction and make sure their values are in expected ranges.

SOLVED: The Glow team solved this finding by adding validation checks to the parameters of the configure_token and configure instructions, ensuring they can only be updated with expected and meaningful values. Additionally, the oracle price account implementation was refactored, making the validation of the price account in the admin parameter unnecessary.

The instructions liquidate_begin, liquidator_invoke and liquidate_end are used by the authorized liquidators to initiate, execute and end liquidations of unhealthy accounts. However the liquidation process does not guarantee that a liquidated account will be healthy after the liquidation ends. A liquidator can liquidate an account only partially, take a liquidation fee and leave the account unhealthy. This process may be repeated until the liquidated account will be drained.

liquidate_end.rs

pub fn liquidate_end_handler(ctx: Context<LiquidateEnd>) -> Result<()> {
    let mut account = ctx.accounts.margin_account.load_mut()?;
    let start_time = ctx.accounts.liquidation.load()?.state.start_time();

    let timed_out = Clock::get()?.unix_timestamp - start_time >= LIQUIDATION_TIMEOUT;

    if (account.liquidator != ctx.accounts.authority.key()) && !timed_out {
        msg!(
            "Only the liquidator may end the liquidation before the timeout of {} seconds",
            LIQUIDATION_TIMEOUT
        );
        return Err(ErrorCode::UnauthorizedLiquidator.into());
    }

    account.end_liquidation();

    emit!(events::LiquidationEnded {
        margin_account: ctx.accounts.margin_account.key(),
        authority: ctx.accounts.authority.key(),
        timed_out,
    });

    Ok(())
}

margin/src/state/account.rs

pub fn end_liquidation(&mut self) {
    self.liquidator = Pubkey::default();
}

To address this issue, it is recommended to incentivize liquidators to fully resolve unhealthy accounts by allowing them to collect liquidation fees only after the account has been restored to a healthy state.

SOLVED: The Glow team resolved this issue by introducing a mechanism that accumulates the liquidator's fees during the liquidation process. Liquidators can only collect their fees once the account has been restored to a healthy state.

The admin_transfer_position instruction enables the protocol administrator to transfer any position from one MarginAccount to another.

According to the documentation, this functionality is intended as a mechanism for manually resolving issues in the protocol caused by problematic user assets.

However, the instruction does not verify that both MarginAccounts belong to the same owner. As a result, the administrator could potentially transfer positions between accounts owned by different users, which could lead to the loss of user funds.

admin_transfer_position.rs

pub struct AdminTransferPosition<'info> {
    /// The administrative authority
    #[account(address = GOVERNOR_ID)]
    pub authority: Signer<'info>,

    /// The target margin account to move a position into
    #[account(mut)]
    pub target_account: AccountLoader<'info, MarginAccount>,

    /// The source account to move a position out of
    #[account(mut)]
    pub source_account: AccountLoader<'info, MarginAccount>,

    /// The token account to be moved from
    #[account(mut, token::mint = token_mint, token::token_program = token_program)]
    pub source_token_account: InterfaceAccount<'info, TokenAccount>,

    /// The token account to be moved into
    #[account(mut, token::mint = token_mint)]
    pub target_token_account: InterfaceAccount<'info, TokenAccount>,

    pub token_mint: InterfaceAccount<'info, Mint>,

    pub token_program: Interface<'info, TokenInterface>,
}

To address this issue, it is recommended to ensure that both the source and target MarginAccounts belong to the same owner. This safeguard will prevent accidental transfers between accounts owned by different users.

ACKNOWLEDGED: The Glow team acknowledged this finding but chose to maintain the existing implementation. The instruction is intended for scenarios where an account cannot be liquidated or in rare, extreme circumstances.

The set_entry and remove_entry instructions in the Metadata program allow airspace authorities to create or delete metadata accounts. However, these instructions do not verify that the provided metadata account has the correct PDA (Program Derived Address). As a result, any airspace authority can modify or remove any metadata account, even if it is not associated with the corresponding airspace.

This issue could lead to metadata manipulation or data inconsistencies within the system.

metadata/src/lib.rs

#[derive(Accounts)]
#[instruction(len: u16)]
pub struct SetEntry<'info> {
    /// The address paying the rent for the account if additional rent is required
    #[account(mut)]
    pub payer: Signer<'info>,

    /// The account containing the metadata to change
    /// CHECK:
    #[account(mut)]
    pub metadata_account: AccountInfo<'info>,

    /// The authority that must sign to make this change
    pub authority: Signer<'info>,

    /// The airspace that the entry belongs to]
    #[cfg_attr(not(feature = "testing"), account(
        constraint = airspace.authority == authority.key(),
    ))]
    pub airspace: Box<Account<'info, Airspace>>,

    pub system_program: Program<'info, System>,
}

#[derive(Accounts)]
pub struct RemoveEntry<'info> {
    /// The account containing the metadata to change
    /// CHECK: This is safe because we can only mutate accounts that we own, and we are
    /// closing this account. The metadata program only has entries, so this is then
    /// presumed to always be an entry. The risk could be if we close a different type
    /// of account unintentionally.
    #[account(mut)]
    pub metadata_account: AccountInfo<'info>,

    /// The authority that must sign to make this change
    pub authority: Signer<'info>,

    /// The airspace that the entry belongs to
    #[cfg_attr(not(feature = "testing"), account(
        constraint = airspace.authority == authority.key(),
    ))]
    pub airspace: Box<Account<'info, Airspace>>,

    /// The address receiving the rent
    /// CHECK: Some details
    #[account(mut)]
    pub receiver: AccountInfo<'info>,
}

1. Create two margin pools: poolA and poolB with their associated airspaces (airspace1 and airspace2) and different airspace authorities (authority1 and autority2).

2. Use airspace authority1 to remove a metadata account associated with airspace2.

To address this issue it is recommended to verify the address of the metadata account and ensure that it is associated with the expected airspace.

SOLVED: The Glow team solved this finding by verifying that the provided metadata account is correctly linked to the specified airspace using the appropriate seeds for the metadata's PDA.

The instruction airspace_permit_revoke allows to revoke permits. The intended behavior based on the documentation is that "For restricted airspaces, anyone can revoke a permit from a revoked regulator.".

However the instruction incorrectly requires the signature of the airspace authority or a permit issuer and thus does not allow any user to revoke the permit of a revoked regulator.

airspace_permit_revoke.rs

// The airspace authority or issuing regulator is always allowed to revoke
if authority != airspace.authority && authority != permit.issuer {
    return err!(AirspaceErrorCode::PermissionDenied);
}

To address this issue, it is recommended to re-evaluate the requirement stating that "for restricted airspaces, anyone can revoke a permit from a revoked regulator." If this requirement has a valid business justification, the implementation of the airspace_permit_revoke instruction should be adjusted accordingly. However, it is important to consider that allowing unrestricted revocation would enable anyone to close the affected permit accounts and claim the associated rent fees.

SOLVED: The Glow team resolved this finding by aligning the code to the documentation and allowing anyone to revoke a permit if an airspace is restricted and the issuer is revoked.

The withdraw_fees instruction is intended to allow the fee owner to withdraw collected fees. However, it consistently fails due to an incorrect token account authority during the burning of deposit notes. Currently, the authority is set to margin_pool, but it should be set to fee_owner for the instruction to execute successfully.

withdraw_fees.rs

fn burn_note_context(&self) -> CpiContext<'_, '_, '_, 'info, Burn<'info>> {
    CpiContext::new(
        self.pool_token_program.to_account_info(),
        Burn {
            from: self.fee_destination.to_account_info(),
            mint: self.deposit_note_mint.to_account_info(),
            // authority: self.margin_pool.to_account_info(),
            authority: self.fee_owner.to_account_info(),
        },
    )
}

To address this issue, it is recommended to update the deposit notes authority to fee_owner when burning tokens.

SOLVED: The Glow team resolved this finding by setting the correct authority when burning tokens.

The margin pool instruction create_pool uses too many stack allocated variables which causes the following error during program compilation:

Compiling glow-margin-pool v1.0.0 (./glow-v1/programs/margin-pool)
Error: Function _ZN165_$LT$glow_margin_pool..instructions..create_pool..CreatePool$u20$as$u20$anchor_lang..Accounts$LT$glow_margin_pool..instructions..create_pool..CreatePoolBumps$GT$$GT$12try_accounts17ha2829e083fe80a7eE Stack offset of 6896 exceeded max offset of 4096 by 2800 bytes, please minimize large stack variables

Stack overflow can lead to undefined behavior and currently blocks deployment to devnet and mainnet. This issue arises due to multiple factors:

1. Anchor 0.30.1 overuses stack variables – See this issue.

2. Differences in feature activation – Devnet and mainnet lack certain features available on localnet, as detailed here.

To address this issue, it is recommended to reduce the size of stack variables by using the Box type and allocating variables on the heap, using zero_copy or splitting instructions into multiple smaller instructions.

SOLVED: The Glow team resolved this issue by manually initializing accounts instead of using Anchor's init constraint, effectively reducing stack usage.

The configure_tokens instruction allows adding new token configurations or updating existing ones in the tokens whitelist. However, this instruction does not explicitly set the TokenConfig::mint_token_program and TokenConfig::underlying_mint_token_program fields, leaving them at their default values.

As a result, when a user registers a new position using the register_position instruction, the program incorrectly determines the associated token standard. The program compares the TokenConfig::mint_token_program or TokenConfig::underlying_mint_token_program addresses based on the type of asset and checks if they match the address of the Token 2022 program. If they do not match, the AccountPosition::is_token_2022 field is set to 0, indicating that the legacy token program is being used. Since the TokenConfig program addresses are never set, the is_token_2022 field is always set to 0, even when the Token 2022 program was actually used.

While this issue does not pose a security risk, it breaks the existing client code and helper functions that interact with the protocol.

configure_token.rs

config.mint = ctx.accounts.mint.key();
config.airspace = ctx.accounts.airspace.key();
config.underlying_mint = updated_config.underlying_mint;
config.admin = updated_config.admin;
config.token_kind = updated_config.token_kind;
config.value_modifier = updated_config.value_modifier;
config.max_staleness = updated_config.max_staleness;

positions.rs

pub fn new_from_config(
    config: &Account<TokenConfig>,
    mint_decimals: u8,
    address: Pubkey,
    adapter: Pubkey,
) -> Self {
    Self {
        mint: config.mint,
        token_program: match config.token_kind {
            TokenKind::Collateral => config.underlying_mint_token_program,
            TokenKind::Claim => config.mint_token_program,
            TokenKind::AdapterCollateral => config.mint_token_program,
        },

account.rs

if let Some(free_position) = free_position {
    free_position.exponent = -(config.decimals as i16);
    free_position.address = config.address;
    free_position.adapter = config.adapter;
    free_position.kind = config.kind.into_integer();
    free_position.balance = 0;
    free_position.value_modifier = config.value_modifier;
    free_position.max_staleness = config.max_staleness;
    // NIT: This isn't a great way of indicating token support, because what happens if
    // there is token_2026 in future?
    free_position.is_token_2022 = if config.token_program == anchor_spl::token_2022::ID {
        1
    } else {
        0
    };

To address this issue, it is recommended to correctly set the TokenConfig::mint_token_program and TokenConfig::underlying_mint_token_program fields during token configuration in the token_configure instruction. This will enable correct determination of the token program during position registration.

SOLVED: The Glow team resolved this finding by correctly setting the respective TokenConfig fields.

The library function permit_issuer_revoke is a helper function to create the AirspacePermitIssuerRevoke instruction. However this function accidentally creates an instruction with AirspacePermitIssuerCreate data instead of AirspacePermitIssuerRevoke data.

Sending such malformed instruction will cause the instruction to fail. This issue does not have any security implications, but may cause problems for example during testing where the tests will unexpectedly fail.

airspace.rs

pub fn permit_issuer_revoke(&self, issuer: Pubkey) -> Instruction {
    let accounts = glow_airspace::accounts::AirspacePermitIssuerRevoke {
        airspace: self.address,
        authority: self.authority,
        receiver: self.payer,
        issuer_id: self.derive_issuer_id(&issuer),
    }
    .to_account_metas(None);

    Instruction {
        accounts,
        program_id: glow_airspace::ID,
        data: glow_airspace::instruction::AirspacePermitIssuerCreate { issuer }.data(),
    }
}

To address this issue, it is recommended to change the AirspacePermitIssuerCreate data to AirspacePermitIssuerRevoke.

SOLVED: The Glow team resolved this finding by setting the correct instruction data.

The instruction AirspacePermitCreate uses unsafe code to disable lifetime checker. However the use of unsafe code is not necessary in this case. Unsafe code in Solana programs is considered bad practice and should be avoided in order to prevent unexpected behavior.

airspace_permit_create.rs

// If the airspace is not restricted, then any signer can create permits
if airspace.is_restricted && airspace.authority != authority.key() {
    // For a restricted airspace, the optional regulator account needs to be verified
    // to prove that the signer is authorized to create the permit
    let _ = Account::<AirspacePermitIssuerId>::try_from(unsafe {
        // 2024-11-05: The transmute is used to extend lifetimes, as it's otherwise been impossible to solve the lifetime
        // constraints here.
        // This is expected to be resolved in a future Anchor version: https://github.com/coral-xyz/anchor/pull/3340
        std::mem::transmute::<&AccountInfo<'_>, &AccountInfo<'_>>(&ctx.accounts.issuer_id)
    })?;

    // No further checks are necessary, since the address is already verified by anchor,
    // and the account data being valid to deserialize means the permission was granted
}

To address this issue, it is recommended to replace the unsafe code by a safe alternative such as: let = AirspacePermitIssuerId::try_deserialize( &mut &ctx.accounts.issuer_id.data.try_borrow().unwrap()[..], );

This code is safe and will verify that the issuer_id account was correctly initialized as expected.

SOLVED: The Glow team resolved this finding by removing the unsafe code and implementing the recommended solution.

The MarginPool::interest_rate method calculates the current interest rate for loans. It also handles the edge case where the pool is empty and contains zero deposit notes.

Currently, when the pool is empty, the method returns borrow_rate_1, which represents the transition point between the first and second interest rate regimes. However, the correct rate in this scenario should be borrow_rate_0, as it corresponds to the starting point of the first regime.

Although this does not currently cause incorrect results—since the interest rate is multiplied by zero when the pool is empty—it should still be corrected to prevent potential issues in future code updates.

margin-pool/src/state.rs

pub fn interest_rate(&self) -> Number {
    let borrow_1 = Number::from_bps(self.config.borrow_rate_1);

    // Catch the edge case of empty pool
    if self.deposit_notes == 0 {
        return borrow_1;
    }

To address this issue it is recommended to return the borrow_rate_0 interest rate when the pool is empty.

SOLVED: The Glow team resolved this issue by returning the borrow_rate_0 interest rate when the pool is empty.

The liquidate_begin instruction allows a liquidator to initiate a liquidation. As part of its logic, the instruction checks whether the account is already undergoing liquidation.

If the same liquidator attempts to invoke this instruction twice, the program will panic by calling the unreachable!() macro. This macro is typically intended for cases where the compiler cannot infer that a specific branch of code should never be reached.

However, in this scenario, it is entirely possible for the same liquidator to call liquidate_begin twice. Therefore, using the unreachable!() macro is inappropriate.

liquidate_begin.rs

// verify not already being liquidated
match account.liquidator {
    liq if liq == liquidator => {
        // this liquidator has already been set as the active liquidator,
        // so there is nothing to do
        unreachable!();
    }

    liq if liq == Pubkey::default() => {
        // not being liquidated, so claim it
        account.start_liquidation(liquidator);
    }

    _ => {
        // already claimed by some other liquidator
        return Err(ErrorCode::Liquidating.into());
    }
}

To resolve this issue, it is recommended to handle cases where the same liquidator invokes the liquidate_begin instruction twice by either treating it as a no-op or returning a meaningful error message.

SOLVED: The Glow team resolved this finding by ensuring that multiple invocations of the liquidate_begin instruction by the same liquidator have no effect.