Inputs to prove_subchain are unconstrained
Description
The prove_data_commitment makes multiple calls to prove_subchain and merges the results together. The inputs for these calls are generated via the hints mechanism.
let start_block =
builder.add(map_ctx.start_block, map_relative_block_nums.as_vec()[0]);
let last_block = builder.add(
map_ctx.start_block,
map_relative_block_nums.as_vec()[BATCH_SIZE - 1],
);
// The query_end_block = min(batch_end_block, global_end_block) for fetching the data commitment inputs.
// If batch_end_block > global_end_block, data_comm_proof will be filled with dummy values.
// This is because prove_subchain only checks up to global_end_block, and doing so reduces RPC calls.
let batch_end_block = builder.add(last_block, one);
let past_global_end = builder.gt(batch_end_block, global_end_block);
let query_end_block = builder.select(past_global_end, global_end_block, batch_end_block);
// Fetch and read the data commitment inputs.
let mut input_stream = VariableStream::new();
input_stream.write(&start_block);
input_stream.write(&query_end_block);
let data_comm_fetcher = DataCommitmentOffchainInputs::<BATCH_SIZE> {};
let output_stream = builder
.async_hint(input_stream, data_comm_fetcher);
let data_comm_proof = output_stream
.read::<DataCommitmentProofVariable<BATCH_SIZE>>(builder);There are constraints enforced on start_block and batch_end_block before construction. However, once the arguments are constructed in data_comm_proof, they are left unconstrained and passed directly to prove_subchain.
Impact
Since the inputs to prove_subchain are left completely unconstrained, the prover can violate any assumptions made by the function and cause undefined behavior as demonstrated in Findings ref↗ and ref↗.
Recommendations
Add proper validation logic for consistency between the initial arguments to the map block and the constructed data_comm_proof object.
builder.assert_is_equal(start_block, data_comm_proof.start_block_height);
builder.assert_is_equal(query_end_block, data_comm_proof.end_block_height);Remediation
This issue has been acknowledged by Succinct, and a fix was implemented in the following commit: .