Description

A block consists of the following fields:

pub struct Block {
    pub facts: BlockFacts,
    pub finalized: bool,
    pub slot_proposed: u64,
}

pub struct BlockFacts {
    pub prev_state_facts: StateFacts,
    pub next_state_facts: StateFacts,
    pub da_commitment: [u8; 32],
    pub withdrawal_root: [u8; 32],
}

pub struct StateFacts {
    pub app_state_commitment: [u8; 32],
    pub deposit_root: [u8; 32],
    pub last_deposit_index: u64,
    pub last_action_id: u64,
}

The propose_block instruction takes a BlockFacts struct as an argument to propose a new block to the bridge program, and it can only be run by the account set as the operator.

The operator is meant to be untrusted, so the implementation performs some validation on the block to assert that the block is correct.

It currently validates the following:

1. The block is proposed by the operator.

2. The data-availability (DA) fact is finalized.

3. The last deposit (if supplied) hashes to the provided deposit root in the next state facts.

And when a block is finalized via the finalize_block instruction, it performs some additional validation:

1. The previous state facts should match the current state facts.

2. The block has not been previously finalized.

3. The block has existed for enough time (based on challenge_period_slots).

However, the last_deposit account in the propose_block instruction does not have sufficient validation:

#[account(
    seeds = [DEPOSIT_SEED, &facts.next_state_facts.last_deposit_index.to_le_bytes()],
    bump,
)]
pub last_deposit: Option<Account<'info, Deposit>>,

Even if a last deposit does exist, it can still be set to None since the account is wrapped in an Option.

This messes up the validation for last_deposit_index and deposit_root for the next state facts:

match (
    facts.next_state_facts.last_deposit_index,
    facts.next_state_facts.deposit_root,
    &ctx.accounts.last_deposit,
) {
    (0, root, None) if root == [0; 32] => (),
    (_, root, Some(d)) if root == d.hash() => (),
    _ => return err!(BridgeError::InvalidDepositRoot),
};

A block can be proposed with a last_deposit_index set to zero and an empty deposit_root if last_deposit is set to None, even if a last deposit does exist.

In addition, there is no validation for the withdrawal_root field as well as the app_state_commitment, last_action_id, and last_deposit_index fields in the next state facts.

Impact

Invalid values for app_state_commitment, last_action_id, last_deposit_index, and deposit_root can cause errors in off-chain bridge infrastructure that trusts these values to be correct.

An invalid Merkle root for withdrawal_root would allow an attacker to verify any withdrawal proof and drain funds from the bridge. However, this would require a compromised bridge operator to propose and finalize a malicious block.

Recommendations

Implement validation for fields that can be validated on chain.

For example, the last_deposit account should never be None if there has been a previous deposit.

Remediation

The issue has been acknowledged by Layer N, and the team intends for most block validation to happen off chain via manual intervention or a challenge mechanism, as mentioned in Discussion point ref↗.

Some extra checks were added in , and the following comment was provided:

A block is allowed to be proposed covering a smaller range of deposits than available at proposal time.

Off-chain bridge infrastructure that should NOT trust these values to be

correct as they belong to an unfinalized block.

Furthermore, an invalid Merkle root for withdrawal_root would NOT allow an attacker to verify any withdrawal proof and drain funds from the bridge as that would require the block to be finalized.

While finalization is intended for the operator, there is no trust assumption on the signer selecting valid blocks to finalize. There is however a (planned) monetary incentive on the proposer. The validation of a block prior to finalization is done by the contract through proposal and finalization-time checks AND our fraud-proof challenge mechanism (planned) / manual review during longer challenge-period + manual intervention (interim).