Skip to content

Commit

Permalink
Validate AGG_SIG_UNSAFE messages to not end in one of the banned suff…
Browse files Browse the repository at this point in the history
…ixes as part of the condition parsing. Also, move the agg_sig_*_additional_data into ConsensusConstants, rather than being computed on-the-fly. This also requires ConsensusConstants to be passed in to run_block_generator and run_puzzle
  • Loading branch information
arvidn committed May 31, 2024
1 parent 3f8a42b commit adbc763
Show file tree
Hide file tree
Showing 22 changed files with 250 additions and 32 deletions.
1 change: 1 addition & 0 deletions crates/chia-bls/src/cached_bls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
3 changes: 3 additions & 0 deletions crates/chia-consensus/benches/run-generator.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -56,6 +57,7 @@ fn run(c: &mut Criterion) {
&block_refs,
11000000000,
ALLOW_BACKREFS,
&TEST_CONSTANTS,
);
let _ = black_box(conds);
start.elapsed()
Expand All @@ -73,6 +75,7 @@ fn run(c: &mut Criterion) {
&block_refs,
11000000000,
ALLOW_BACKREFS,
&TEST_CONSTANTS,
);
let _ = black_box(conds);
start.elapsed()
Expand Down
3 changes: 3 additions & 0 deletions crates/chia-consensus/fuzz/fuzz_targets/fast-forward.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -90,6 +91,7 @@ fn test_ff(
spend.coin.amount,
11000000000,
0,
&TEST_CONSTANTS,
);

// run new spend
Expand All @@ -101,6 +103,7 @@ fn test_ff(
new_coin.amount,
11000000000,
0,
&TEST_CONSTANTS,
);

// These are the kinds of failures that can happen because of the
Expand Down
2 changes: 2 additions & 0 deletions crates/chia-consensus/fuzz/fuzz_targets/parse-conditions.rs
Original file line number Diff line number Diff line change
@@ -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,
};
Expand Down Expand Up @@ -81,6 +82,7 @@ fuzz_target!(|data: &[u8]| {
input,
*flags,
&mut max_cost,
&TEST_CONSTANTS,
&mut visitor,
);
}
Expand Down
3 changes: 2 additions & 1 deletion crates/chia-consensus/fuzz/fuzz_targets/parse-spends.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -23,6 +24,6 @@ fuzz_target!(|data: &[u8]| {
ENABLE_SOFTFORK_CONDITION,
ENABLE_MESSAGE_CONDITIONS,
] {
let _ret = parse_spends::<MempoolVisitor>(&a, input, 33000000000, *flags);
let _ret = parse_spends::<MempoolVisitor>(&a, input, 33000000000, *flags, &TEST_CONSTANTS);
}
});
2 changes: 2 additions & 0 deletions crates/chia-consensus/fuzz/fuzz_targets/process-spend.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![no_main]
use chia_consensus::consensus_constants::TEST_CONSTANTS;
use chia_consensus::gen::conditions::{
process_single_spend, MempoolVisitor, ParseState, SpendBundleConditions,
};
Expand Down Expand Up @@ -30,6 +31,7 @@ fuzz_target!(|data: &[u8]| {
conds,
*flags,
&mut cost_left,
&TEST_CONSTANTS,
);
}
});
12 changes: 10 additions & 2 deletions crates/chia-consensus/fuzz/fuzz_targets/run-generator.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -9,8 +10,14 @@ use libfuzzer_sys::fuzz_target;

fuzz_target!(|data: &[u8]| {
let mut a1 = make_allocator(LIMIT_HEAP);
let r1 =
run_block_generator::<&[u8], MempoolVisitor>(&mut a1, data, &[], 110000000, ALLOW_BACKREFS);
let r1 = run_block_generator::<&[u8], MempoolVisitor>(
&mut a1,
data,
&[],
110000000,
ALLOW_BACKREFS,
&TEST_CONSTANTS,
);
drop(a1);

let mut a2 = make_allocator(LIMIT_HEAP);
Expand All @@ -20,6 +27,7 @@ fuzz_target!(|data: &[u8]| {
&[],
110000000,
ALLOW_BACKREFS,
&TEST_CONSTANTS,
);
drop(a2);

Expand Down
2 changes: 2 additions & 0 deletions crates/chia-consensus/fuzz/fuzz_targets/run-puzzle.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -22,5 +23,6 @@ fuzz_target!(|data: &[u8]| {
spend.coin.amount,
11000000000,
ALLOW_BACKREFS,
&TEST_CONSTANTS,
);
});
27 changes: 26 additions & 1 deletion crates/chia-consensus/src/consensus_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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"
)),
Expand Down
3 changes: 3 additions & 0 deletions crates/chia-consensus/src/fast_forward.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,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;
Expand Down Expand Up @@ -234,6 +235,7 @@ mod tests {
spend.coin.amount,
11000000000,
0,
&TEST_CONSTANTS,
)
.expect("run_puzzle");

Expand All @@ -246,6 +248,7 @@ mod tests {
new_coin.amount,
11000000000,
0,
&TEST_CONSTANTS,
)
.expect("run_puzzle");

Expand Down
50 changes: 49 additions & 1 deletion crates/chia-consensus/src/gen/conditions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -253,6 +254,43 @@ 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);
if buf
.as_ref()
.ends_with(constants.agg_sig_me_additional_data.as_ref())
|| buf
.as_ref()
.ends_with(constants.agg_sig_parent_additional_data.as_ref())
|| buf
.as_ref()
.ends_with(constants.agg_sig_puzzle_additional_data.as_ref())
|| buf
.as_ref()
.ends_with(constants.agg_sig_amount_additional_data.as_ref())
|| buf
.as_ref()
.ends_with(constants.agg_sig_puzzle_amount_additional_data.as_ref())
|| buf
.as_ref()
.ends_with(constants.agg_sig_parent_amount_additional_data.as_ref())
|| buf
.as_ref()
.ends_with(constants.agg_sig_parent_puzzle_additional_data.as_ref())
{
Err(ValidationErr(msg, ErrorCode::InvalidMessage))
} else {
Ok(())
}
}

fn maybe_check_args_terminator(
a: &Allocator,
arg: NodePtr,
Expand Down Expand Up @@ -830,6 +868,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)?;
Expand Down Expand Up @@ -869,6 +908,7 @@ pub fn process_single_spend<V: SpendVisitor>(
conditions,
flags,
max_cost,
constants,
&mut visitor,
)
}
Expand Down Expand Up @@ -914,6 +954,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;
Expand Down Expand Up @@ -1182,6 +1223,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));
}
Expand Down Expand Up @@ -1269,6 +1313,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();
Expand All @@ -1295,6 +1340,7 @@ pub fn parse_spends<V: SpendVisitor>(
conds,
flags,
&mut cost_left,
constants,
)?;
}

Expand Down Expand Up @@ -1486,6 +1532,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;
Expand Down Expand Up @@ -1725,7 +1773,7 @@ fn cond_test_cb(
print!("{:02x}", c);
}
println!();
match parse_spends::<MempoolVisitor>(&a, n, 11000000000, flags) {
match parse_spends::<MempoolVisitor>(&a, n, 11000000000, flags, &TEST_CONSTANTS) {
Ok(list) => {
for n in &list.spends {
println!("{:?}", n);
Expand Down
6 changes: 5 additions & 1 deletion crates/chia-consensus/src/gen/run_block_generator.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::consensus_constants::ConsensusConstants;
use crate::gen::conditions::{
parse_spends, process_single_spend, validate_conditions, ParseState, SpendBundleConditions,
};
Expand Down Expand Up @@ -43,6 +44,7 @@ pub fn run_block_generator<GenBuf: AsRef<[u8]>, V: SpendVisitor>(
block_refs: &[GenBuf],
max_cost: u64,
flags: u32,
constants: &ConsensusConstants,
) -> Result<SpendBundleConditions, ValidationErr> {
let mut cost_left = max_cost;
let byte_cost = program.len() as u64 * COST_PER_BYTE;
Expand Down Expand Up @@ -76,7 +78,7 @@ pub fn run_block_generator<GenBuf: AsRef<[u8]>, V: SpendVisitor>(

// we pass in what's left of max_cost here, to fail early in case the
// cost of a condition brings us over the cost limit
let mut result = parse_spends::<V>(a, generator_output, cost_left, flags)?;
let mut result = parse_spends::<V>(a, generator_output, cost_left, flags, constants)?;
result.cost += max_cost - cost_left;
Ok(result)
}
Expand Down Expand Up @@ -116,6 +118,7 @@ pub fn run_block_generator2<GenBuf: AsRef<[u8]>, V: SpendVisitor>(
block_refs: &[GenBuf],
max_cost: u64,
flags: u32,
constants: &ConsensusConstants,
) -> Result<SpendBundleConditions, ValidationErr> {
let byte_cost = program.len() as u64 * COST_PER_BYTE;

Expand Down Expand Up @@ -181,6 +184,7 @@ pub fn run_block_generator2<GenBuf: AsRef<[u8]>, V: SpendVisitor>(
conditions,
flags,
&mut cost_left,
constants,
)?;
}
if a.atom_len(all_spends) != 0 {
Expand Down
4 changes: 4 additions & 0 deletions crates/chia-consensus/src/gen/run_puzzle.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::consensus_constants::ConsensusConstants;
use crate::gen::conditions::{parse_conditions, ParseState, Spend, SpendBundleConditions};
use crate::gen::flags::ALLOW_BACKREFS;
use crate::gen::spend_visitor::SpendVisitor;
Expand All @@ -12,6 +13,7 @@ use clvmr::run_program::run_program;
use clvmr::serde::{node_from_bytes, node_from_bytes_backrefs};
use std::sync::Arc;

#[allow(clippy::too_many_arguments)]
pub fn run_puzzle<V: SpendVisitor>(
a: &mut Allocator,
puzzle: &[u8],
Expand All @@ -20,6 +22,7 @@ pub fn run_puzzle<V: SpendVisitor>(
amount: u64,
max_cost: u64,
flags: u32,
constants: &ConsensusConstants,
) -> Result<SpendBundleConditions, ValidationErr> {
let deserialize = if (flags & ALLOW_BACKREFS) != 0 {
node_from_bytes_backrefs
Expand Down Expand Up @@ -67,6 +70,7 @@ pub fn run_puzzle<V: SpendVisitor>(
conditions,
flags,
&mut cost_left,
constants,
&mut visitor,
)?;
ret.cost = max_cost - cost_left;
Expand Down
Loading

0 comments on commit adbc763

Please sign in to comment.