NGL Bridge + Gorples Bridge - Entangle Labs

The Initialize statement of the gorples-bridge program allows the initialization of the bridge config account. This requires two parameters: bridge_router_address and min_bridge_amount.

initialize.rs

pub struct Initialize<'info> {
    /// Bridge admin
    #[account(mut, signer)]
    pub admin: Signer<'info>,

    /// Bridge config
    #[account(init, payer = admin, space = Config::LEN, seeds = [BRIDGE_ROOT, b"CONFIG"], bump)]
    pub config: Box<Account<'info, Config>>,

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

pub fn handle_initialize(
    ctx: Context<Initialize>,
    bridge_router_address: Vec<u8>,
    min_bridge_amount: u64,
) -> Result<()> {
    ctx.accounts.config.bridge_router_address = bridge_router_address.try_into().unwrap();
    ctx.accounts.config.admin = ctx.accounts.admin.key();
    ctx.accounts.config.min_bridge_amount = min_bridge_amount;
    Ok(())
}

The SetBridgeConfig instruction enables the bridge config administrator to modify the bridge config by providing values for the same fields as in the initialization, along with an additional fee_collector_vault address.

set_bridge_config.rs

pub struct SetBridgeConfig<'info> {
    /// Bridge admin
    #[account(mut, signer, address = config.admin)]
    pub admin: Signer<'info>,

    /// Bridge config
    #[account(seeds = [BRIDGE_ROOT, b"CONFIG"], bump)]
    pub config: Box<Account<'info, Config>>,
}

pub fn handle_set_bridge_config(
    ctx: Context<SetBridgeConfig>,
    bridge_router_address: Vec<u8>,
    min_bridge_amount: u64,
    fee_collector_vault: Pubkey,
) -> Result<()> {
    ctx.accounts.config.bridge_router_address = bridge_router_address.try_into().unwrap();
    ctx.accounts.config.min_bridge_amount = min_bridge_amount;
    ctx.accounts.config.fee_collector_vault = fee_collector_vault;
    Ok(())

The fee_collector_vault is intended to be the account where fees obtained in photon_msg will be collected. However, this address, like the other two, is not validated. As a result, it is possible to provide an invalid account or a token account without the corresponding mint associated to match the mint of the core config. This issue could prevent fee collection in photon_msg, causing the user to be unable to receive mined tokens, effectively resulting in a denial of service for this instruction until a valid fee_collector_vault is assigned.

photon_msg.rs

    /// User vault for token
    #[account(
        init_if_needed,
        payer = executor,
        associated_token::authority = user,
        associated_token::mint = mint,
        associated_token::token_program = token_program
    )]
    pub user_vault: Box<InterfaceAccount<'info, TokenAccount>>,

    /// Fee collector vault for token
    #[account(
        mut,
        address = config.fee_collector_vault,
        token::mint = mint,
        token::token_program = token_program
    )]
    pub fee_collector_vault: Box<InterfaceAccount<'info, TokenAccount>>,

By default, this scenario occurs if SetBridgeConfig is not called to assign a correct address, as the zero address is assigned during initialization.

In the ngl-bridge program, this address is also not validated but is provided during initialization and managed by the deployer, thus mitigating the level of security risk. However, since gorples-bridge is more susceptible to front-running attacks as it is explained in HAL-01, the following security risks are possible:

• Invalid fee_collector_vault addresses could lead to uncollected fees and disrupted operations.

• Users may experience denial of service until the configuration is corrected.

• Security risks increase if malicious actors exploit the non-validated fields.

1. Initialize bridge config

2. Initialize Core Config

3. Set bridge config with an invalid account as fee collector vault or with a token account whose mint does not match the core config mint

To address this issue, it is recommended to implement the following measures:

• Include the fee_collector_vault address as a parameter at initialization to prevent it from being assigned to the default address.

• Add a validation to ensure that fee_collector_vault is a token account with an associated mint that matches the mint in the core configuration.

Remediation Plan

PARTIALLY SOLVED: The Entangle Labs team partially solved this issue by modifying the initialization instruction handler. Now, it requires the fee_collector_vault address to be provided as a parameter, ensuring it is not initialized by default. This change ensures that the address is properly configured. Following the remediation plan for HAL_01, the deployer is responsible for ensuring and providing a valid and correct address.

The Initialize instruction of the ngl-core program allows the deployer to initialize the core config account. To achieve this, the bridge_authority address must be provided as a parameter along with other values. However, this address is not validated, which could lead to the accidental setting of an unintentional or invalid address. Since the ngl-core program does not have a functionality to change the core config account values (except for setting a new admin via SetAdmin), this mistake would be irreversible.

This could result in denial of service for mining and token burning operations if the bridge_authority was invalid, or misuse of Mint operations if an incorrect address was assigned.

lib.rs (ngl-core)

    pub fn initialize(
        ctx: Context<Initialize>,
        bridge_authority: Pubkey,
        name: String,
        symbol: String,
        uri: String,
    ) -> Result<()> {
        ctx.accounts.config.admin = ctx.accounts.admin.key();
        ctx.accounts.config.contracts[0] = bridge_authority;
        ctx.accounts.config.mint = ctx.accounts.mint.key();

#[derive(Accounts)]
pub struct MintToken<'info> {
    /// Mint authority
    /// CHECK: not loaded
    #[account(
        signer,
        constraint = 
            mint_authority.key() != Pubkey::default() && config.contracts.contains(&mint_authority.key())
                @ CustomError::Unauthorized
    )]
    pub mint_authority: Signer<'info>,

    /// Token authority
    /// CHECK: not loaded
    #[account(seeds = [ROOT, b"AUTHORITY"], bump)]
    pub authority: UncheckedAccount<'info>,

lib.rs (ngl-core)

#[derive(Accounts)]
pub struct BurnToken<'info> {
    #[account(signer)]
    pub vault_owner: Signer<'info>,

    /// Burn authority
    /// CHECK: not loaded
    #[account(
        signer,
        constraint = 
            burn_authority.key() != Pubkey::default() && config.contracts.contains(&burn_authority.key())
                @ CustomError::Unauthorized
    )]
    pub burn_authority: Signer<'info>,

On the other hand, the Bridge and PhotonMsg instructions use as mint_authority and burn_authority the token authority, so if this authority is not the one provided as bridge_authority in the initialization of the core config, it will result in a denial of service in both instructions.

bridge.rs

pub struct Bridge<'info> {
    /// Sender wallet
    #[account(signer)]
    pub sender: Signer<'info>,

    /// Token authority
    /// CHECK: not loaded
    #[account(seeds = [ROOT, b"AUTHORITY"], bump)]
    pub authority: AccountInfo<'info>,

    /// Token mint
    #[account(
        mut,
        address = core_config.mint @ CustomError::InvalidMint,
        mint::token_program = token_program
    )]
    pub mint: Box<InterfaceAccount<'info, Mint>>,

    /// Sender vault
    #[account(
        mut,
        token::mint = mint,
        token::authority = sender,
        token::token_program = token_program
    )]
    pub sender_vault: Box<InterfaceAccount<'info, TokenAccount>>,

    /// Bridge config
    #[account(seeds = [ROOT, b"CONFIG"], bump)]
    pub config: Box<Account<'info, Config>>,

    /// Core config
    pub core_config: Box<Account<'info, ngl_core::Config>>,

   require_gte!(amount, ctx.accounts.config.min_bridge_amount, CustomError::AmountTooLow);
    let bump = &[ctx.bumps.authority];
    let seed = &[ROOT, &b"AUTHORITY"[..], bump][..];
    let seeds = &[seed];
    // Burn
    let accounts = BurnToken {
        vault_owner: ctx.accounts.sender.to_account_info(),
        burn_authority: ctx.accounts.authority.to_account_info(),
        config: ctx.accounts.core_config.to_account_info(),
        mint: ctx.accounts.mint.to_account_info(),
        vault: ctx.accounts.sender_vault.to_account_info(),
        token_program: ctx.accounts.token_program.to_account_info(),
    };

photon_msg.rs

pub struct PhotonMsg<'info> {
    /// Executor wallet
    #[account(mut, signer)]
    pub executor: Signer<'info>,

    /// Protocol call authority (from photon program)
    #[account(
        signer,
        seeds = [entangle_photon_sol/*photon*/::photon::ROOT, b"CALL_AUTHORITY", NGL_PROTOCOL_ID],
        bump,
        seeds::program = Photon::id()
    )]
    pub call_authority: Signer<'info>,

    /// Provided by photon program
    pub op_info: Box<Account<'info, OpInfo>>,

    /// Token authority
    /// CHECK: not loaded
    #[account(seeds = [ROOT, b"AUTHORITY"], bump)]
    pub authority: UncheckedAccount<'info>,

    /// Token mint (checked by core)
    #[account(mut)]
    pub mint: Box<InterfaceAccount<'info, Mint>>,

    /// CHECK: User wallet
    pub user: UncheckedAccount<'info>,

    /// User vault for token
    #[account(
        init_if_needed,
        payer = executor,
        associated_token::authority = user,
        associated_token::mint = mint,
        associated_token::token_program = token_program
    )]
    pub user_vault: Box<InterfaceAccount<'info, TokenAccount>>,

    /// Fee collector vault for token
    #[account(
        mut,
        address = config.fee_collector_vault,
        token::mint = mint,
        token::token_program = token_program
    )]
    pub fee_collector_vault: Box<InterfaceAccount<'info, TokenAccount>>,

    /// Bridge config
    #[account(seeds = [ROOT, b"CONFIG"], bump)]
    pub config: Box<Account<'info, Config>>,

    /// Core config
    /// CHECK: by core program
    pub core_config: UncheckedAccount<'info>,

 let accounts = MintToken {
                mint_authority: ctx.accounts.authority.to_account_info(),
                authority: ctx.accounts.core_authority.to_account_info(),
                config: ctx.accounts.core_config.to_account_info(),
                mint: ctx.accounts.mint.to_account_info(),
                vault: ctx.accounts.fee_collector_vault.to_account_info(),
                token_program: ctx.accounts.token_program.to_account_info(),
            };
            let cpi_ctx = CpiContext::new_with_signer(
                ctx.accounts.core_program.to_account_info(),
                accounts,
                seeds,
            );
            mint_token(cpi_ctx, fee)?;
            // Mint for user
            let bump = &[ctx.bumps.authority];
            let seed = &[ROOT, &b"AUTHORITY"[..], bump][..];
            let seeds = &[seed];
            let accounts = MintToken {
                mint_authority: ctx.accounts.authority.to_account_info(),
                authority: ctx.accounts.core_authority.to_account_info(),
                config: ctx.accounts.core_config.to_account_info(),
                mint: ctx.accounts.mint.to_account_info(),
                vault: ctx.accounts.user_vault.to_account_info(),
                token_program: ctx.accounts.token_program.to_account_info(),
            };

Conversely, in the initialize instruction handler of the ngl-bridge program, the values passed by parameters are also not validated. However, this program includes a SetBridgeConfig instruction that allows the administrator (initially the deployer) to correct any values of the bridge config at any time in case of an error, although those values are not validated in it either.

initialize.rs (ngl-bridge)

#[derive(Accounts)]
pub struct Initialize<'info> {
    /// Bridge admin
    #[account(mut, signer, address = DEPLOYER.parse().expect("Deployer key not set"))]
    pub admin: Signer<'info>,

    /// Bridge config
    #[account(init, payer = admin, space = Config::LEN, seeds = [ROOT, b"CONFIG"], bump)]
    pub config: Box<Account<'info, Config>>,

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

pub fn handle_initialize(
    ctx: Context<Initialize>,
    bridge_router_address: Vec<u8>,
    min_bridge_amount: u64,
    fee_collector_vault: Pubkey,
) -> Result<()> {
    ctx.accounts.config.bridge_router_address = bridge_router_address.try_into().unwrap();
    ctx.accounts.config.admin = ctx.accounts.admin.key();
    ctx.accounts.config.min_bridge_amount = min_bridge_amount;
    ctx.accounts.config.fee_collector_vault = fee_collector_vault;
    Ok(())

To address this issue, the following modifications are recommended:

• Change the authority used in bridge and photon_msg of the ngl-bridge program that will be used as burn_authority and mint_authority to be a pda with the corresponding bridge authority seeds.

• Add a check in the initialize instruction handler of the ngl-core program to verify that the provided bridge_authority address is not an invalid address and that it corresponds to the expected bridge_authority pda which should match the one from the previous point.

It is also recommended to validate the values passed by parameter in the instruction handler initialize and set_bridge_config of the ngl-bridge program to check that they are valid to avoid a situation like the one described in HAL-01 and HAL-02 by error.

Remediation Plan

RISK ACCEPTED: The Entangle Labs team accepted the risk of this finding.

The SetAdmin instruction in the ngl-bridge and ngl-core programs allows the deployer to set a new config admin once the config is initialized, provided as a parameter. This new admin will hold critical privileges for managing the config account. However, the provided admin address is not validated to ensure it is a valid address. Additionally, the instruction handler uses a one-step procedure for admin delegation.

set_admin.rs

pub struct SetAdmin<'info> {
    /// Deployer address
    #[account(signer, address = DEPLOYER.parse().expect("Deployer key not set"))]
    pub deployer: Signer<'info>,

    /// Bridge config
    #[account(mut, seeds = [ROOT, b"CONFIG"], bump)]
    pub config: Box<Account<'info, Config>>,
}

pub fn handle_set_admin(ctx: Context<SetAdmin>, admin: Pubkey) -> Result<()> {
    ctx.accounts.config.admin = admin;
    Ok(())
}

lib.rs

    pub fn set_admin(ctx: Context<SetAdmin>, admin: Pubkey) -> Result<()> {
        ctx.accounts.config.admin = admin;
        Ok(())
    }

This may lead to issues where the MintByAdmin and SetBridgeConfig statements cannot be called if the admin address is invalid. Moreover, mistakenly setting an incorrect address could allow unauthorized use of high privileges by the new malicious admin, posing security risks until the deployer calls SetAdmin again to set a valid and trusted administrator.

Although this issue does not pose an immediate security risk, as the deployer can reassign a new config admin, it is advisable to verify that the new admin address provided is valid to prevent potential issues.

Remediation Plan

ACKNOWLEDGED: The Entangle Labs team acknowledged this finding.