Description

The following assumption in get_data_commitment is wrong.

// If end_block < start_block, then this data commitment will be marked as disabled, and the
// output of this function is not used. Therefore, the logic assumes
// nb_blocks is always positive.

The batch is disabled if and only if start_block >= global_end_block, but this function is actually called with batch_end, which can still be lower. This means you can end up with a case where end_block > start_block.

While this implicit assumption has no significant impact by itself, it can be chained with another unenforced assumption in get_data_commitment:

let nb_blocks_in_batch = self.sub(end_block, start_block);
// Note: nb_blocks_in_batch is assumed to be less than 2^32 (which is a reasonable
// assumption for any data commitment as in practice, the number of blocks in a data
// commitment range will be much smaller than 2^32). This is fine as
// nb_blocks_in_batch.limbs[1] is unused.
let nb_enabled_leaves = nb_blocks_in_batch.limbs[0].variable;

The number of enabled leaves is directly taken from the first limb, assuming the second to be zero. However, in the case where end_block > start_block, nb_blocks_in_batch can have an integer underflow.

Impact

A malicous prover can supply an arbitrary value for batch_end_block that is less than batch_start_block, which causes nb_enabled_leaves to underflow and lead to undefined behavior.

This vulnerability is however only exploitable when combined with another vulnerability such as finding ref↗ or finding ref↗, and is therefore marked as low impact.

Recommendations

Consider checking that end_block > start_block explicitly in get_data_commitment:

let is_valid = self.gt(end_block, start_block);
self.assert_is_equal(is_valid, true_bool);

Additionally, we recommend explicitly constraining the second limb to 0 as a sanity check

self.assert_is_equal(nb_blocks_in_batch.limbs[1].variable, self.constant::<U64Variable>(0u64));

Remediation

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