From 88b09c812a6ac7bc3de9ec9b7155b9ca6910be70 Mon Sep 17 00:00:00 2001 From: Conrado Gouvea Date: Thu, 18 Nov 2021 15:06:07 -0300 Subject: [PATCH] Check nSpendsSapling, nOutputsSapling, and nActionsOrchard 2^16 limit (#3069) * Check nSpendsSapling, nOutputsSapling, and nActionsOrchard 2^16 limit * Apply suggestions from code review Co-authored-by: Janito Vaqueiro Ferreira Filho * Removed not required #[macro_use] Co-authored-by: Janito Vaqueiro Ferreira Filho --- Cargo.lock | 1 + zebra-chain/Cargo.toml | 1 + zebra-chain/src/orchard/shielded_data.rs | 9 ++++++++- zebra-chain/src/sapling/output.rs | 10 +++++++++- zebra-chain/src/sapling/spend.rs | 10 +++++++++- 5 files changed, 28 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 732fad37..d9d2da0b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4347,6 +4347,7 @@ dependencies = [ "serde-big-array", "sha2", "spandoc", + "static_assertions", "subtle", "thiserror", "tracing", diff --git a/zebra-chain/Cargo.toml b/zebra-chain/Cargo.toml index 2c89bba5..70868a47 100644 --- a/zebra-chain/Cargo.toml +++ b/zebra-chain/Cargo.toml @@ -40,6 +40,7 @@ secp256k1 = { version = "0.20.3", features = ["serde"] } serde = { version = "1", features = ["serde_derive", "rc"] } serde-big-array = "0.3.2" sha2 = { version = "0.9.8", features=["compress"] } +static_assertions = "1.1.0" subtle = "2.4" thiserror = "1" uint = "0.9.1" diff --git a/zebra-chain/src/orchard/shielded_data.rs b/zebra-chain/src/orchard/shielded_data.rs index 366dec12..e09fba50 100644 --- a/zebra-chain/src/orchard/shielded_data.rs +++ b/zebra-chain/src/orchard/shielded_data.rs @@ -182,7 +182,14 @@ impl TrustedPreallocate for Action { // Since a serialized Vec uses at least one byte for its length, // and the signature is required, // a valid max allocation can never exceed this size - (MAX_BLOCK_BYTES - 1) / AUTHORIZED_ACTION_SIZE + const MAX: u64 = (MAX_BLOCK_BYTES - 1) / AUTHORIZED_ACTION_SIZE; + // > [NU5 onward] nSpendsSapling, nOutputsSapling, and nActionsOrchard MUST all be less than 2^16. + // https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus + // This acts as nActionsOrchard and is therefore subject to the rule. + // The maximum value is actually smaller due to the block size limit, + // but we ensure the 2^16 limit with a static assertion. + static_assertions::const_assert!(MAX < (1 << 16)); + MAX } } diff --git a/zebra-chain/src/sapling/output.rs b/zebra-chain/src/sapling/output.rs index 175dca4f..9c897745 100644 --- a/zebra-chain/src/sapling/output.rs +++ b/zebra-chain/src/sapling/output.rs @@ -217,7 +217,15 @@ impl TrustedPreallocate for OutputInTransactionV4 { fn max_allocation() -> u64 { // Since a serialized Vec uses at least one byte for its length, // the max allocation can never exceed (MAX_BLOCK_BYTES - 1) / OUTPUT_SIZE - (MAX_BLOCK_BYTES - 1) / OUTPUT_SIZE + const MAX: u64 = (MAX_BLOCK_BYTES - 1) / OUTPUT_SIZE; + // > [NU5 onward] nSpendsSapling, nOutputsSapling, and nActionsOrchard MUST all be less than 2^16. + // https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus + // This acts as nOutputsSapling and is therefore subject to the rule. + // The maximum value is actually smaller due to the block size limit, + // but we ensure the 2^16 limit with a static assertion. + // (The check is not required pre-NU5, but it doesn't cause problems.) + static_assertions::const_assert!(MAX < (1 << 16)); + MAX } } diff --git a/zebra-chain/src/sapling/spend.rs b/zebra-chain/src/sapling/spend.rs index 000515d9..6dac34bd 100644 --- a/zebra-chain/src/sapling/spend.rs +++ b/zebra-chain/src/sapling/spend.rs @@ -276,7 +276,15 @@ impl TrustedPreallocate for SpendPrefixInTransactionV5 { // Since a serialized Vec uses at least one byte for its length, // and the associated fields are required, // a valid max allocation can never exceed this size - (MAX_BLOCK_BYTES - 1) / SHARED_ANCHOR_SPEND_SIZE + const MAX: u64 = (MAX_BLOCK_BYTES - 1) / SHARED_ANCHOR_SPEND_SIZE; + // > [NU5 onward] nSpendsSapling, nOutputsSapling, and nActionsOrchard MUST all be less than 2^16. + // https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus + // This acts as nSpendsSapling and is therefore subject to the rule. + // The maximum value is actually smaller due to the block size limit, + // but we ensure the 2^16 limit with a static assertion. + // (The check is not required pre-NU5, but it doesn't cause problems.) + static_assertions::const_assert!(MAX < (1 << 16)); + MAX } }