Merge pull request #554 from Chia-Network/agg-sig-suffix

Validate AGG_SIG_UNSAFE messages to not end in one of the banned suffixes
Chia-Network · Jun 4, 2024 · a521169 · a521169
2 parents 9ca0a04 + e2944be
commit a521169
Show file tree

Hide file tree

Showing 23 changed files with 293 additions and 35 deletions.
diff --git a/crates/chia-bls/src/cached_bls.rs b/crates/chia-bls/src/cached_bls.rs
@@ -95,6 +95,7 @@ mod python {
     #[pymethods]
     impl BlsCache {
         #[new]
+        #[pyo3(signature = (size=None))]
         pub fn init(size: Option<u32>) -> PyResult<Self> {
             let Some(size) = size else {
                 return Ok(Self::default());

diff --git a/crates/chia-consensus/benches/run-generator.rs b/crates/chia-consensus/benches/run-generator.rs
@@ -1,3 +1,4 @@
+use chia_consensus::consensus_constants::TEST_CONSTANTS;
 use chia_consensus::gen::conditions::MempoolVisitor;
 use chia_consensus::gen::flags::ALLOW_BACKREFS;
 use chia_consensus::gen::run_block_generator::{run_block_generator, run_block_generator2};
@@ -56,6 +57,7 @@ fn run(c: &mut Criterion) {
                         &block_refs,
                         11_000_000_000,
                         ALLOW_BACKREFS,
+                        &TEST_CONSTANTS,
                     );
                     let _ = black_box(conds);
                     start.elapsed()
@@ -73,6 +75,7 @@ fn run(c: &mut Criterion) {
                         &block_refs,
                         11_000_000_000,
                         ALLOW_BACKREFS,
+                        &TEST_CONSTANTS,
                     );
                     let _ = black_box(conds);
                     start.elapsed()

diff --git a/crates/chia-consensus/fuzz/fuzz_targets/fast-forward.rs b/crates/chia-consensus/fuzz/fuzz_targets/fast-forward.rs
@@ -1,4 +1,5 @@
 #![no_main]
+use chia_consensus::consensus_constants::TEST_CONSTANTS;
 use chia_consensus::fast_forward::fast_forward_singleton;
 use chia_consensus::gen::conditions::{MempoolVisitor, ELIGIBLE_FOR_FF};
 use chia_consensus::gen::run_puzzle::run_puzzle;
@@ -90,6 +91,7 @@ fn test_ff(
         spend.coin.amount,
         11_000_000_000,
         0,
+        &TEST_CONSTANTS,
     );
 
     // run new spend
@@ -101,6 +103,7 @@ fn test_ff(
         new_coin.amount,
         11_000_000_000,
         0,
+        &TEST_CONSTANTS,
     );
 
     // These are the kinds of failures that can happen because of the

diff --git a/crates/chia-consensus/fuzz/fuzz_targets/parse-conditions.rs b/crates/chia-consensus/fuzz/fuzz_targets/parse-conditions.rs
@@ -1,6 +1,7 @@
 #![no_main]
 use libfuzzer_sys::fuzz_target;
 
+use chia_consensus::consensus_constants::TEST_CONSTANTS;
 use chia_consensus::gen::conditions::{
     parse_conditions, MempoolVisitor, ParseState, Spend, SpendBundleConditions,
 };
@@ -81,6 +82,7 @@ fuzz_target!(|data: &[u8]| {
             input,
             *flags,
             &mut max_cost,
+            &TEST_CONSTANTS,
             &mut visitor,
         );
     }

diff --git a/crates/chia-consensus/fuzz/fuzz_targets/parse-spends.rs b/crates/chia-consensus/fuzz/fuzz_targets/parse-spends.rs
@@ -5,6 +5,7 @@ use chia_consensus::gen::conditions::{parse_spends, MempoolVisitor};
 use clvmr::{Allocator, NodePtr};
 use fuzzing_utils::{make_list, BitCursor};
 
+use chia_consensus::consensus_constants::TEST_CONSTANTS;
 use chia_consensus::gen::flags::{
     COND_ARGS_NIL, ENABLE_MESSAGE_CONDITIONS, ENABLE_SOFTFORK_CONDITION, NO_UNKNOWN_CONDS,
     STRICT_ARGS_COUNT,
@@ -23,6 +24,7 @@ fuzz_target!(|data: &[u8]| {
         ENABLE_SOFTFORK_CONDITION,
         ENABLE_MESSAGE_CONDITIONS,
     ] {
-        let _ret = parse_spends::<MempoolVisitor>(&a, input, 33_000_000_000, *flags);
+        let _ret =
+            parse_spends::<MempoolVisitor>(&a, input, 33_000_000_000, *flags, &TEST_CONSTANTS);
     }
 });
diff --git a/crates/chia-consensus/fuzz/fuzz_targets/process-spend.rs b/crates/chia-consensus/fuzz/fuzz_targets/process-spend.rs
@@ -1,4 +1,5 @@
 #![no_main]
+use chia_consensus::consensus_constants::TEST_CONSTANTS;
 use chia_consensus::gen::conditions::{
     process_single_spend, MempoolVisitor, ParseState, SpendBundleConditions,
 };
@@ -30,6 +31,7 @@ fuzz_target!(|data: &[u8]| {
             conds,
             *flags,
             &mut cost_left,
+            &TEST_CONSTANTS,
         );
     }
 });
diff --git a/crates/chia-consensus/fuzz/fuzz_targets/run-generator.rs b/crates/chia-consensus/fuzz/fuzz_targets/run-generator.rs
@@ -1,5 +1,6 @@
 #![no_main]
 use chia_consensus::allocator::make_allocator;
+use chia_consensus::consensus_constants::TEST_CONSTANTS;
 use chia_consensus::gen::conditions::MempoolVisitor;
 use chia_consensus::gen::flags::ALLOW_BACKREFS;
 use chia_consensus::gen::run_block_generator::{run_block_generator, run_block_generator2};
@@ -15,6 +16,7 @@ fuzz_target!(|data: &[u8]| {
         &[],
         110_000_000,
         ALLOW_BACKREFS,
+        &TEST_CONSTANTS,
     );
     drop(a1);
 
@@ -25,6 +27,7 @@ fuzz_target!(|data: &[u8]| {
         &[],
         110_000_000,
         ALLOW_BACKREFS,
+        &TEST_CONSTANTS,
     );
     drop(a2);
 

diff --git a/crates/chia-consensus/fuzz/fuzz_targets/run-puzzle.rs b/crates/chia-consensus/fuzz/fuzz_targets/run-puzzle.rs
@@ -1,4 +1,5 @@
 #![no_main]
+use chia_consensus::consensus_constants::TEST_CONSTANTS;
 use chia_consensus::gen::conditions::MempoolVisitor;
 use chia_consensus::gen::flags::ALLOW_BACKREFS;
 use chia_consensus::gen::run_puzzle::run_puzzle;
@@ -22,5 +23,6 @@ fuzz_target!(|data: &[u8]| {
         spend.coin.amount,
         11_000_000_000,
         ALLOW_BACKREFS,
+        &TEST_CONSTANTS,
     );
 });
diff --git a/crates/chia-consensus/src/consensus_constants.rs b/crates/chia-consensus/src/consensus_constants.rs
@@ -74,8 +74,15 @@ pub struct ConsensusConstants {
     /// We override this value based on the chain being run (testnet0, testnet1, mainnet, etc).
     genesis_challenge: Bytes32,
 
-    /// Forks of chia should change this value to provide replay attack protection.
+    /// Forks of chia should change these values to provide replay attack protection.
     agg_sig_me_additional_data: Bytes32,
+    /// By convention, the below additional data is derived from the agg_sig_me_additional_data
+    agg_sig_parent_additional_data: Bytes32,
+    agg_sig_puzzle_additional_data: Bytes32,
+    agg_sig_amount_additional_data: Bytes32,
+    agg_sig_puzzle_amount_additional_data: Bytes32,
+    agg_sig_parent_amount_additional_data: Bytes32,
+    agg_sig_parent_puzzle_additional_data: Bytes32,
 
     /// The block at height must pay out to this pool puzzle hash.
     genesis_pre_farm_pool_puzzle_hash: Bytes32,
@@ -163,6 +170,24 @@ pub const TEST_CONSTANTS: ConsensusConstants = ConsensusConstants {
     agg_sig_me_additional_data: Bytes32::new(hex!(
         "ccd5bb71183532bff220ba46c268991a3ff07eb358e8255a65c30a2dce0e5fbb"
     )),
+    agg_sig_parent_additional_data: Bytes32::new(hex!(
+        "baf5d69c647c91966170302d18521b0a85663433d161e72c826ed08677b53a74"
+    )),
+    agg_sig_puzzle_additional_data: Bytes32::new(hex!(
+        "284fa2ef486c7a41cc29fc99c9d08376161e93dd37817edb8219f42dca7592c4"
+    )),
+    agg_sig_amount_additional_data: Bytes32::new(hex!(
+        "cda186a9cd030f7a130fae45005e81cae7a90e0fa205b75f6aebc0d598e0348e"
+    )),
+    agg_sig_puzzle_amount_additional_data: Bytes32::new(hex!(
+        "0f7d90dff0613e6901e24dae59f1e690f18b8f5fbdcf1bb192ac9deaf7de22ad"
+    )),
+    agg_sig_parent_amount_additional_data: Bytes32::new(hex!(
+        "585796bd90bb553c0430b87027ffee08d88aba0162c6e1abbbcc6b583f2ae7f9"
+    )),
+    agg_sig_parent_puzzle_additional_data: Bytes32::new(hex!(
+        "2ebfdae17b29d83bae476a25ea06f0c4bd57298faddbbc3ec5ad29b9b86ce5df"
+    )),
     genesis_pre_farm_pool_puzzle_hash: Bytes32::new(hex!(
         "d23da14695a188ae5708dd152263c4db883eb27edeb936178d4d988b8f3ce5fc"
     )),

diff --git a/crates/chia-consensus/src/fast_forward.rs b/crates/chia-consensus/src/fast_forward.rs
@@ -155,6 +155,7 @@ pub fn fast_forward_singleton(
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::consensus_constants::TEST_CONSTANTS;
     use crate::gen::conditions::MempoolVisitor;
     use crate::gen::run_puzzle::run_puzzle;
     use chia_protocol::CoinSpend;
@@ -233,6 +234,7 @@ mod tests {
             spend.coin.amount,
             11_000_000_000,
             0,
+            &TEST_CONSTANTS,
         )
         .expect("run_puzzle");
 
@@ -245,6 +247,7 @@ mod tests {
             new_coin.amount,
             11_000_000_000,
             0,
+            &TEST_CONSTANTS,
         )
         .expect("run_puzzle");
 

diff --git a/crates/chia-consensus/src/gen/conditions.rs b/crates/chia-consensus/src/gen/conditions.rs
@@ -16,6 +16,7 @@ use super::opcodes::{
 };
 use super::sanitize_int::{sanitize_uint, SanitizedUint};
 use super::validation_error::{first, next, rest, ErrorCode, ValidationErr};
+use crate::consensus_constants::ConsensusConstants;
 use crate::gen::flags::{
     AGG_SIG_ARGS, COND_ARGS_NIL, DISALLOW_INFINITY_G1, NO_RELATIVE_CONDITIONS_ON_EPHEMERAL,
     NO_UNKNOWN_CONDS, STRICT_ARGS_COUNT,
@@ -238,6 +239,31 @@ pub enum Condition {
     SkipRelativeCondition,
 }
 
+fn check_agg_sig_unsafe_message(
+    a: &Allocator,
+    msg: NodePtr,
+    constants: &ConsensusConstants,
+) -> Result<(), ValidationErr> {
+    if a.atom_len(msg) < 32 {
+        return Ok(());
+    }
+    let buf = a.atom(msg);
+    for additional_data in &[
+        constants.agg_sig_me_additional_data.as_ref(),
+        constants.agg_sig_parent_additional_data.as_ref(),
+        constants.agg_sig_puzzle_additional_data.as_ref(),
+        constants.agg_sig_amount_additional_data.as_ref(),
+        constants.agg_sig_puzzle_amount_additional_data.as_ref(),
+        constants.agg_sig_parent_amount_additional_data.as_ref(),
+        constants.agg_sig_parent_puzzle_additional_data.as_ref(),
+    ] {
+        if buf.as_ref().ends_with(additional_data) {
+            return Err(ValidationErr(msg, ErrorCode::InvalidMessage));
+        }
+    }
+    Ok(())
+}
+
 fn maybe_check_args_terminator(
     a: &Allocator,
     arg: NodePtr,
@@ -817,6 +843,7 @@ pub fn process_single_spend<V: SpendVisitor>(
     conditions: NodePtr,
     flags: u32,
     max_cost: &mut Cost,
+    constants: &ConsensusConstants,
 ) -> Result<(), ValidationErr> {
     let parent_id = sanitize_hash(a, parent_id, 32, ErrorCode::InvalidParentId)?;
     let puzzle_hash = sanitize_hash(a, puzzle_hash, 32, ErrorCode::InvalidPuzzleHash)?;
@@ -856,6 +883,7 @@ pub fn process_single_spend<V: SpendVisitor>(
         conditions,
         flags,
         max_cost,
+        constants,
         &mut visitor,
     )
 }
@@ -901,6 +929,7 @@ pub fn parse_conditions<V: SpendVisitor>(
     mut iter: NodePtr,
     flags: u32,
     max_cost: &mut Cost,
+    constants: &ConsensusConstants,
     visitor: &mut V,
 ) -> Result<(), ValidationErr> {
     let mut announce_countdown: u32 = 1024;
@@ -1169,6 +1198,9 @@ pub fn parse_conditions<V: SpendVisitor>(
                 }
             }
             Condition::AggSigUnsafe(pk, msg) => {
+                // AGG_SIG_UNSAFE messages are not allowed to end with the
+                // suffix added to other AGG_SIG_* conditions
+                check_agg_sig_unsafe_message(a, msg, constants)?;
                 if let Some(pk) = to_key(a, pk, flags)? {
                     ret.agg_sig_unsafe.push((pk, msg));
                 }
@@ -1256,6 +1288,7 @@ pub fn parse_spends<V: SpendVisitor>(
     spends: NodePtr,
     max_cost: Cost,
     flags: u32,
+    constants: &ConsensusConstants,
 ) -> Result<SpendBundleConditions, ValidationErr> {
     let mut ret = SpendBundleConditions::default();
     let mut state = ParseState::default();
@@ -1282,6 +1315,7 @@ pub fn parse_spends<V: SpendVisitor>(
             conds,
             flags,
             &mut cost_left,
+            constants,
         )?;
     }
 
@@ -1473,6 +1507,8 @@ fn u64_to_bytes(n: u64) -> Vec<u8> {
     buf
 }
 #[cfg(test)]
+use crate::consensus_constants::TEST_CONSTANTS;
+#[cfg(test)]
 use crate::gen::flags::ENABLE_SOFTFORK_CONDITION;
 #[cfg(test)]
 use clvmr::number::Number;
@@ -1713,7 +1749,7 @@ fn cond_test_cb(
         print!("{c:02x}");
     }
     println!();
-    match parse_spends::<MempoolVisitor>(&a, n, 11_000_000_000, flags) {
+    match parse_spends::<MempoolVisitor>(&a, n, 11_000_000_000, flags, &TEST_CONSTANTS) {
         Ok(list) => {
             for n in &list.spends {
                 println!("{n:?}");
@@ -3474,7 +3510,7 @@ fn test_agg_sig_unsafe_invalid_pubkey() {
 }
 
 #[test]
-fn test_agg_sig_unsafe_invalid_msg() {
+fn test_agg_sig_unsafe_long_msg() {
     // AGG_SIG_UNSAFE
     assert_eq!(
         cond_test("((({h1} ({h2} (123 (((49 ({pubkey} ({longmsg} )))))")
@@ -3484,6 +3520,40 @@ fn test_agg_sig_unsafe_invalid_msg() {
     );
 }
 
+#[cfg(test)]
+#[rstest]
+// these are the suffixes used for AGG_SIG_* conditions (other than
+// AGG_SIG_UNSAFE)
+#[case("0xccd5bb71183532bff220ba46c268991a3ff07eb358e8255a65c30a2dce0e5fbb")]
+#[case("0xbaf5d69c647c91966170302d18521b0a85663433d161e72c826ed08677b53a74")]
+#[case("0x284fa2ef486c7a41cc29fc99c9d08376161e93dd37817edb8219f42dca7592c4")]
+#[case("0xcda186a9cd030f7a130fae45005e81cae7a90e0fa205b75f6aebc0d598e0348e")]
+#[case("0x0f7d90dff0613e6901e24dae59f1e690f18b8f5fbdcf1bb192ac9deaf7de22ad")]
+#[case("0x585796bd90bb553c0430b87027ffee08d88aba0162c6e1abbbcc6b583f2ae7f9")]
+#[case("0x2ebfdae17b29d83bae476a25ea06f0c4bd57298faddbbc3ec5ad29b9b86ce5df")]
+// The same suffixes, but 1 byte prepended
+#[case("0x01ccd5bb71183532bff220ba46c268991a3ff07eb358e8255a65c30a2dce0e5fbb")]
+#[case("0x01baf5d69c647c91966170302d18521b0a85663433d161e72c826ed08677b53a74")]
+#[case("0x01284fa2ef486c7a41cc29fc99c9d08376161e93dd37817edb8219f42dca7592c4")]
+#[case("0x01cda186a9cd030f7a130fae45005e81cae7a90e0fa205b75f6aebc0d598e0348e")]
+#[case("0x010f7d90dff0613e6901e24dae59f1e690f18b8f5fbdcf1bb192ac9deaf7de22ad")]
+#[case("0x01585796bd90bb553c0430b87027ffee08d88aba0162c6e1abbbcc6b583f2ae7f9")]
+#[case("0x012ebfdae17b29d83bae476a25ea06f0c4bd57298faddbbc3ec5ad29b9b86ce5df")]
+fn test_agg_sig_unsafe_invalid_msg(
+    #[case] msg: &str,
+    #[values(43, 44, 45, 46, 47, 48, 49, 50)] opcode: u16,
+) {
+    let ret = cond_test_flag(
+        format!("((({{h1}} ({{h2}} (123 ((({opcode} ({{pubkey}} ({msg} )))))").as_str(),
+        ENABLE_SOFTFORK_CONDITION,
+    );
+    if opcode == AGG_SIG_UNSAFE {
+        assert_eq!(ret.unwrap_err().1, ErrorCode::InvalidMessage);
+    } else {
+        assert!(ret.is_ok());
+    }
+}
+
 #[test]
 fn test_agg_sig_unsafe_exceed_cost() {
     // AGG_SIG_UNSAFE