From d954e67238a9637a738fb395719cc972f6c287f1 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Sat, 25 Mar 2023 01:36:28 -0400 Subject: [PATCH] Ensure InInstruction data is properly limited Bitcoin didn't check, assuming data was <= 80 bytes thanks to being in OP_RETURN. An additional global check has been added. --- processor/src/coins/bitcoin.rs | 3 ++- processor/src/main.rs | 14 ++++++++++++-- substrate/serai/client/src/coins/bitcoin.rs | 4 ++-- substrate/serai/primitives/src/lib.rs | 7 ++++--- 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/processor/src/coins/bitcoin.rs b/processor/src/coins/bitcoin.rs index 311b8440..1b6a7ffd 100644 --- a/processor/src/coins/bitcoin.rs +++ b/processor/src/coins/bitcoin.rs @@ -36,7 +36,7 @@ use bitcoin_serai::bitcoin::{ PackedLockTime, Sequence, Script, Witness, TxIn, TxOut, Address as BAddress, }; -use serai_client::coins::bitcoin::Address; +use serai_client::{primitives::MAX_DATA_LEN, coins::bitcoin::Address}; use crate::{ coins::{ @@ -357,6 +357,7 @@ impl Coin for Bitcoin { } else { vec![] }; + data.truncate(MAX_DATA_LEN.try_into().unwrap()); outputs.push(Output { kind, output, data }) } diff --git a/processor/src/main.rs b/processor/src/main.rs index ce275e79..4b56a018 100644 --- a/processor/src/main.rs +++ b/processor/src/main.rs @@ -19,7 +19,7 @@ use tokio::time::sleep; use scale::Decode; use serai_client::{ - primitives::{Amount, WithAmount}, + primitives::{MAX_DATA_LEN, Amount, WithAmount}, tokens::primitives::OutInstruction, in_instructions::primitives::{Shorthand, RefundableInInstruction}, }; @@ -425,7 +425,17 @@ async fn run(raw_db: D, coin: C, mut coordinato return None; } - let shorthand = Shorthand::decode(&mut output.data()).ok()?; + let data = output.data(); + if data.len() > MAX_DATA_LEN { + error!( + "data in output {} exceeded MAX_DATA_LEN ({MAX_DATA_LEN}): {}", + hex::encode(output.id()), + data.len(), + ); + data = data[.. MAX_DATA_LEN]; + } + + let shorthand = Shorthand::decode(&mut data).ok()?; let instruction = RefundableInInstruction::try_from(shorthand).ok()?; // TODO2: Set instruction.origin if not set (and handle refunds in general) Some(WithAmount { data: instruction.instruction, amount: Amount(output.amount()) }) diff --git a/substrate/serai/client/src/coins/bitcoin.rs b/substrate/serai/client/src/coins/bitcoin.rs index cedca9ab..f8a37245 100644 --- a/substrate/serai/client/src/coins/bitcoin.rs +++ b/substrate/serai/client/src/coins/bitcoin.rs @@ -53,8 +53,8 @@ impl TryFrom> for Address { EncodedAddress::P2WSH(hash) => { Payload::WitnessProgram { version: WitnessVersion::V0, program: hash.to_vec() } } - EncodedAddress::P2TR(hash) => { - Payload::WitnessProgram { version: WitnessVersion::V1, program: hash.to_vec() } + EncodedAddress::P2TR(key) => { + Payload::WitnessProgram { version: WitnessVersion::V1, program: key.to_vec() } } }, })) diff --git a/substrate/serai/primitives/src/lib.rs b/substrate/serai/primitives/src/lib.rs index 861eeb85..52ad990e 100644 --- a/substrate/serai/primitives/src/lib.rs +++ b/substrate/serai/primitives/src/lib.rs @@ -27,9 +27,10 @@ pub use balance::*; mod account; pub use account::*; -// Monero, our current longest address candidate, has a longest address of featured with payment ID -// 1 (enum) + 1 (flags) + 64 (two keys) + 8 (payment ID) = 74 -pub const MAX_ADDRESS_LEN: u32 = 74; +// Monero, our current longest address candidate, has a longest address of featured +// 1 (enum) + 1 (flags) + 64 (two keys) = 66 +// When JAMTIS arrives, it'll become 114 bytes +pub const MAX_ADDRESS_LEN: u32 = 128; #[derive(Clone, PartialEq, Eq, Debug, Encode, Decode, MaxEncodedLen, TypeInfo)] #[cfg_attr(feature = "std", derive(Serialize, Deserialize))]