From 34c7a27c2aa92b20e1ea0c416c02675508bd18b3 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 1 Sep 2021 02:50:47 +1000 Subject: [PATCH] Security: Replace queued checkpoint blocks with duplicate hashes (#2697) We don't check the authorizing data hash until checkpoint blocks reach the state. So signatures, proofs, or scripts could be different, even if the block hash is the same. Co-authored-by: Conrado Gouvea --- zebra-consensus/src/checkpoint.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/zebra-consensus/src/checkpoint.rs b/zebra-consensus/src/checkpoint.rs index 663f58136..965b66813 100644 --- a/zebra-consensus/src/checkpoint.rs +++ b/zebra-consensus/src/checkpoint.rs @@ -539,13 +539,23 @@ where .entry(height) .or_insert_with(|| QueuedBlockList::with_capacity(1)); - // Replace older requests by newer ones by swapping the oneshot. + // Replace older requests with newer ones. + // The newer block is ok, the older block is an error. for qb in qblocks.iter_mut() { if qb.block.hash == hash { let e = VerifyCheckpointError::NewerRequest { height, hash }; tracing::trace!(?e, "failing older of duplicate requests"); - let old_tx = std::mem::replace(&mut qb.tx, new_qblock.tx); - let _ = old_tx.send(Err(e)); + + // ## Security + // + // Replace the entire queued block. + // + // We don't check the authorizing data hash until checkpoint blocks reach the state. + // So signatures, proofs, or scripts could be different, + // even if the block hash is the same. + + let old = std::mem::replace(qb, new_qblock); + let _ = old.tx.send(Err(e)); return Ok(req_block); } }