Description

The prove_subchain processes blocks in the range from batch_start_block (inclusive) to min(batch_start_block + BATCH_SIZE, global_end_block) (exclusive). The end of the range given here arises from two facts: processing stops with the block at height global_end_block, and the loop starts at batch_start_block and runs for BATCH_SIZE iterations.

However, when calling get_data_commitment, the range passed is the one from batch_start_block to min(batch_end_block, global_end_block):

let is_less_than_target = self.lte(batch_end_block, *global_end_block);
  let end_block_num = self.select(is_less_than_target, batch_end_block, *global_end_block);
  let data_merkle_root = self.get_data_commitment::<BATCH_SIZE>(
      &data_comm_proof.data_hashes,
      batch_start_block,
      end_block_num,
  );

If get_data_commitment is passed a range larger than the maximum number of leaves, in this case BATCH_SIZE, then the return value will be the same as if the range had width equal to the number of leaves. Thus, in practice, the range used to calculate the Merkle root here is the one from batch_start_block to min(batch_start_block + BATCH_SIZE, batch_end_block, global_end_block).

For the range used for processing and the range used to construct the data-commitment Merkle tree to be the same, it is thus required that batch_end_block >= min(batch_start_block + BATCH_SIZE, global_end_block). Thus, if prove_subchain is called with arguments for which batch_end_block < min(batch_start_block + BATCH_SIZE, global_end_block), then the header hash chain will be verified for a different range than the range used to calculate the corresponding data_merkle_root.

Impact

A malicous prover can supply an arbitrary value for batch_end_block which is less than min(batch_start_block + BATCH_SIZE, global_end_block), which allows for the batch_end_header_hash and data_merkle_root to correspond to different blocks in the on-chain state.

This vulnerability is however only exploitable if inputs to the prove_subchain function are not constrained properly as in finding ref↗ and therefore marked as informational.

Recommendations

This can be mitigated by adding a constraint to enforce a batch is of the proper length.

let nb_blocks = self.sub(batch_end_block, batch_start_block);
self.assert_is_equal(nb_blocks, BATCH_SIZE);

Remediation

This issue has been acknowledged by Succinct, and a fix was implemented in the following commit: .