Merkle verification could be bypassed
Description
As per the sync-protocol specifications, the update.next_sync_committee should be validated via the Merkle proof if the next_sync_committee_branch is not empty, as shown below:
if not is_sync_committee_update(update):
assert update.next_sync_committee == SyncCommittee()
else:
if update_attested_period == store_period and is_next_sync_committee_known(store):
assert update.next_sync_committee == store.next_sync_committee
assert is_valid_normalized_merkle_branch(
leaf=hash_tree_root(update.next_sync_committee),
branch=update.next_sync_committee_branch,
gindex=next_sync_committee_gindex_at_slot(update.attested_header.beacon.slot)
root=update.attested_header.beacon.state_root,
)But in the implementation of ethereum-light-client, the Merkle verification is executed in the case update.next_sync_committee and trusted_consensus_state.next_sync_committee() are both available. As the trusted_consensus_state.next_sync_committee() (see Finding ref↗) comes from user input too, it might be possible to prove the incorrect next_sync_committee.
if let (Some(next_sync_committee), Some(stored_next_sync_committee)) = (
&update.next_sync_committee,
trusted_consensus_state.next_sync_committee(),
) {
if update_attested_period == stored_period {
ensure!(
next_sync_committee == stored_next_sync_committee,
EthereumIBCError::NextSyncCommitteeMismatch {
expected: stored_next_sync_committee.aggregate_pubkey,
found: next_sync_committee.aggregate_pubkey,
}
);
}
// This validates the given next sync committee against the attested header's state root.
validate_merkle_branch(
next_sync_committee.tree_hash_root(),
update.next_sync_committee_branch.unwrap_or_default().into(),
NEXT_SYNC_COMMITTEE_BRANCH_DEPTH,
get_subtree_index(NEXT_SYNC_COMMITTEE_INDEX),
update.attested_header.beacon.state_root,
)
.map_err(|e| EthereumIBCError::ValidateNextSyncCommitteeFailed(Box::new(e)))?;
}Impact
As the sync committee is used to validate the attested_header, an incorrect header could be provided, which could be used to prove an incorrect finalized_root and state root. This could be used to update the light client with incorrect states.
Recommendations
We recommend following the specification and validating the Merkle proof in cases similar to how it is validated in the specification.
Remediation
This issue has been acknowledged by Interchain Labs, and a fix was implemented in commit d46a8e55↗.