From d611337394e5e2c2fb7a292a4e4849238d8717f1 Mon Sep 17 00:00:00 2001 From: Justin Starry Date: Fri, 13 Nov 2020 16:20:50 +0800 Subject: [PATCH] Fix overflow in entry hash count verification --- ledger/src/entry.rs | 125 +++++++++++++++++++++++++++++++++----------- 1 file changed, 95 insertions(+), 30 deletions(-) diff --git a/ledger/src/entry.rs b/ledger/src/entry.rs index 23036e2af1..e0b58daeb7 100644 --- a/ledger/src/entry.rs +++ b/ledger/src/entry.rs @@ -622,7 +622,7 @@ impl EntrySlice for [Entry] { } for entry in self { - *tick_hash_count += entry.num_hashes; + *tick_hash_count = tick_hash_count.saturating_add(entry.num_hashes); if entry.is_tick() { if *tick_hash_count != hashes_per_tick { warn!( @@ -900,52 +900,117 @@ mod tests { fn test_verify_tick_hash_count() { let hashes_per_tick = 10; let tx = Transaction::default(); - let tx_entry = Entry::new(&Hash::default(), 1, vec![tx]); - let full_tick_entry = Entry::new_tick(hashes_per_tick, &Hash::default()); - let partial_tick_entry = Entry::new_tick(hashes_per_tick - 1, &Hash::default()); + + let no_hash_tx_entry = Entry { + transactions: vec![tx.clone()], + ..Entry::default() + }; + let single_hash_tx_entry = Entry { + transactions: vec![tx.clone()], + num_hashes: 1, + ..Entry::default() + }; + let partial_tx_entry = Entry { + num_hashes: hashes_per_tick - 1, + transactions: vec![tx.clone()], + ..Entry::default() + }; + let full_tx_entry = Entry { + num_hashes: hashes_per_tick, + transactions: vec![tx.clone()], + ..Entry::default() + }; + let max_hash_tx_entry = Entry { + transactions: vec![tx.clone()], + num_hashes: u64::MAX, + ..Entry::default() + }; + let no_hash_tick_entry = Entry::new_tick(0, &Hash::default()); let single_hash_tick_entry = Entry::new_tick(1, &Hash::default()); + let partial_tick_entry = Entry::new_tick(hashes_per_tick - 1, &Hash::default()); + let full_tick_entry = Entry::new_tick(hashes_per_tick, &Hash::default()); + let max_hash_tick_entry = Entry::new_tick(u64::MAX, &Hash::default()); - let no_ticks = vec![]; + // empty batch should succeed if hashes_per_tick hasn't been reached let mut tick_hash_count = 0; - assert!(no_ticks.verify_tick_hash_count(&mut tick_hash_count, hashes_per_tick)); + let mut entries = vec![]; + assert!(entries.verify_tick_hash_count(&mut tick_hash_count, hashes_per_tick)); assert_eq!(tick_hash_count, 0); - // validation is disabled when hashes_per_tick == 0 - let no_hash_tick = vec![no_hash_tick_entry.clone()]; - assert!(no_hash_tick.verify_tick_hash_count(&mut tick_hash_count, 0)); - assert_eq!(tick_hash_count, 0); - - // validation is disabled when hashes_per_tick == 0 - let tx_and_no_hash_tick = vec![tx_entry.clone(), no_hash_tick_entry]; - assert!(tx_and_no_hash_tick.verify_tick_hash_count(&mut tick_hash_count, 0)); - assert_eq!(tick_hash_count, 0); - - let single_tick = vec![full_tick_entry]; - assert!(single_tick.verify_tick_hash_count(&mut tick_hash_count, hashes_per_tick)); - assert_eq!(tick_hash_count, 0); - assert!(!single_tick.verify_tick_hash_count(&mut tick_hash_count, hashes_per_tick - 1)); + // empty batch should fail if hashes_per_tick has been reached + tick_hash_count = hashes_per_tick; + assert!(!entries.verify_tick_hash_count(&mut tick_hash_count, hashes_per_tick)); assert_eq!(tick_hash_count, hashes_per_tick); tick_hash_count = 0; - let ticks_and_txs = vec![tx_entry.clone(), partial_tick_entry.clone()]; - assert!(ticks_and_txs.verify_tick_hash_count(&mut tick_hash_count, hashes_per_tick)); + // validation is disabled when hashes_per_tick == 0 + entries = vec![max_hash_tx_entry.clone()]; + assert!(entries.verify_tick_hash_count(&mut tick_hash_count, 0)); assert_eq!(tick_hash_count, 0); - let partial_tick = vec![partial_tick_entry]; - assert!(!partial_tick.verify_tick_hash_count(&mut tick_hash_count, hashes_per_tick)); + // partial tick should fail + entries = vec![partial_tick_entry.clone()]; + assert!(!entries.verify_tick_hash_count(&mut tick_hash_count, hashes_per_tick)); assert_eq!(tick_hash_count, hashes_per_tick - 1); tick_hash_count = 0; - let tx_entries: Vec = (0..hashes_per_tick - 1).map(|_| tx_entry.clone()).collect(); - let tx_entries_and_tick = [tx_entries, vec![single_hash_tick_entry]].concat(); - assert!(tx_entries_and_tick.verify_tick_hash_count(&mut tick_hash_count, hashes_per_tick)); + // full tick entry should succeed + entries = vec![no_hash_tx_entry, full_tick_entry.clone()]; + assert!(entries.verify_tick_hash_count(&mut tick_hash_count, hashes_per_tick)); assert_eq!(tick_hash_count, 0); - let too_many_tx_entries: Vec = - (0..hashes_per_tick).map(|_| tx_entry.clone()).collect(); - assert!(!too_many_tx_entries.verify_tick_hash_count(&mut tick_hash_count, hashes_per_tick)); + // oversized tick entry should fail + assert!(!entries.verify_tick_hash_count(&mut tick_hash_count, hashes_per_tick - 1)); assert_eq!(tick_hash_count, hashes_per_tick); + tick_hash_count = 0; + + // partial tx entry without tick entry should succeed + entries = vec![partial_tx_entry.clone()]; + assert!(entries.verify_tick_hash_count(&mut tick_hash_count, hashes_per_tick)); + assert_eq!(tick_hash_count, hashes_per_tick - 1); + tick_hash_count = 0; + + // full tx entry with tick entry should succeed + entries = vec![full_tx_entry.clone(), no_hash_tick_entry.clone()]; + assert!(entries.verify_tick_hash_count(&mut tick_hash_count, hashes_per_tick)); + assert_eq!(tick_hash_count, 0); + + // full tx entry with oversized tick entry should fail + entries = vec![full_tx_entry.clone(), single_hash_tick_entry.clone()]; + assert!(!entries.verify_tick_hash_count(&mut tick_hash_count, hashes_per_tick)); + assert_eq!(tick_hash_count, hashes_per_tick + 1); + tick_hash_count = 0; + + // full tx entry without tick entry should fail + entries = vec![full_tx_entry]; + assert!(!entries.verify_tick_hash_count(&mut tick_hash_count, hashes_per_tick)); + assert_eq!(tick_hash_count, hashes_per_tick); + tick_hash_count = 0; + + // tx entry and a tick should succeed + entries = vec![single_hash_tx_entry.clone(), partial_tick_entry.clone()]; + assert!(entries.verify_tick_hash_count(&mut tick_hash_count, hashes_per_tick)); + assert_eq!(tick_hash_count, 0); + + // many tx entries and a tick should succeed + let tx_entries: Vec = (0..hashes_per_tick - 1) + .map(|_| single_hash_tx_entry.clone()) + .collect(); + entries = [tx_entries, vec![single_hash_tick_entry]].concat(); + assert!(entries.verify_tick_hash_count(&mut tick_hash_count, hashes_per_tick)); + assert_eq!(tick_hash_count, 0); + + // check overflow saturation should fail + entries = vec![full_tick_entry.clone(), max_hash_tick_entry]; + assert!(!entries.verify_tick_hash_count(&mut tick_hash_count, hashes_per_tick)); + assert_eq!(tick_hash_count, u64::MAX); + tick_hash_count = 0; + + // check overflow saturation should fail + entries = vec![max_hash_tx_entry.clone(), full_tick_entry]; + assert!(!entries.verify_tick_hash_count(&mut tick_hash_count, hashes_per_tick)); + assert_eq!(tick_hash_count, u64::MAX); } #[test]