diff --git a/lib/state/block.rs b/lib/state/block.rs index 549f2bf..09d5708 100644 --- a/lib/state/block.rs +++ b/lib/state/block.rs @@ -84,6 +84,21 @@ pub fn validate( }; return Err(err); } + // The authorization and address checks below pair authorizations with + // spent UTXOs positionally via `zip`, which silently ignores any trailing + // inputs when there are fewer authorizations than inputs requiring one. + // Require an exact count so that every input requiring authorization is + // covered by a provided authorization. + let n_authorizations_required: usize = filled_txs + .iter() + .map(|t| t.spent_utxos_requiring_auth().len()) + .sum(); + if body.authorizations.len() != n_authorizations_required { + return Err(Error::WrongNumberOfAuthorizations { + expected: n_authorizations_required, + received: body.authorizations.len(), + }); + } let spent_utxos = filled_txs .iter() .flat_map(|t| t.spent_utxos_requiring_auth().into_iter()); @@ -168,6 +183,21 @@ pub fn prevalidate( }; return Err(err); } + // The authorization and address checks below pair authorizations with + // spent UTXOs positionally via `zip`, which silently ignores any trailing + // inputs when there are fewer authorizations than inputs requiring one. + // Require an exact count so that every input requiring authorization is + // covered by a provided authorization. + let n_authorizations_required: usize = filled_transactions + .iter() + .map(|t| t.spent_utxos_requiring_auth().len()) + .sum(); + if body.authorizations.len() != n_authorizations_required { + return Err(Error::WrongNumberOfAuthorizations { + expected: n_authorizations_required, + received: body.authorizations.len(), + }); + } let spent_utxos = filled_transactions .iter() .flat_map(|t| t.spent_utxos_requiring_auth().into_iter()); diff --git a/lib/state/error.rs b/lib/state/error.rs index 2c7a8e9..1d16103 100644 --- a/lib/state/error.rs +++ b/lib/state/error.rs @@ -265,6 +265,10 @@ pub enum Error { UtxoDoubleSpent, #[error(transparent)] WithdrawalBundle(#[from] WithdrawalBundleError), + #[error( + "wrong number of authorizations: expected {expected}, but received {received}" + )] + WrongNumberOfAuthorizations { expected: usize, received: usize }, #[error("wrong public key for address")] WrongPubKeyForAddress, } diff --git a/lib/state/mod.rs b/lib/state/mod.rs index fe2fb85..c50d1ab 100644 --- a/lib/state/mod.rs +++ b/lib/state/mod.rs @@ -484,6 +484,17 @@ impl State { ) -> Result { let filled_transaction = self.fill_transaction(rotxn, &transaction.transaction)?; + // Pairing authorizations with spent UTXOs via `zip` silently ignores + // trailing inputs when there are too few authorizations. Require an + // exact count so that every input requiring authorization is covered. + let n_authorizations_required = + filled_transaction.spent_utxos_requiring_auth().len(); + if transaction.authorizations.len() != n_authorizations_required { + return Err(Error::WrongNumberOfAuthorizations { + expected: n_authorizations_required, + received: transaction.authorizations.len(), + }); + } for (authorization, spent_utxo) in transaction .authorizations .iter() @@ -650,3 +661,111 @@ impl Watchable<()> for State { tokio_stream::wrappers::WatchStream::new(self.tip.watch().clone()) } } + +#[cfg(test)] +mod tests { + use ed25519_dalek::SigningKey; + + use super::{Error, State}; + use crate::{ + authorization, + types::{ + Address, AuthorizedTransaction, BitcoinOutputContent, FilledOutput, + FilledOutputContent, OutPoint, OutPointKey, Output, OutputContent, + Transaction, VerifyingKey, + }, + }; + + /// Open a fresh state in a unique temporary directory. + fn new_state() -> (sneed::Env, State) { + let path = std::env::temp_dir() + .join(format!("plain_bitnames_auth_count_{}", std::process::id())); + let _remove_result = std::fs::remove_dir_all(&path); + std::fs::create_dir_all(&path).unwrap(); + let env = { + let mut opts = heed::EnvOpenOptions::new(); + opts.map_size(10 * 1024 * 1024).max_dbs(State::NUM_DBS); + unsafe { sneed::Env::open(&opts, &path) }.unwrap() + }; + let state = State::new(&env).unwrap(); + (env, state) + } + + /// Fund `address` with a single bitcoin UTXO of `value` sats, returning its + /// outpoint. + fn fund( + env: &sneed::Env, + state: &State, + address: Address, + value: u64, + ) -> OutPoint { + let outpoint = OutPoint::Regular { + txid: Default::default(), + vout: 0, + }; + let output = FilledOutput::new( + address, + FilledOutputContent::Bitcoin(BitcoinOutputContent( + bitcoin::Amount::from_sat(value), + )), + ); + let mut rwtxn = env.write_txn().unwrap(); + state + .utxos + .put(&mut rwtxn, &OutPointKey::from(&outpoint), &output) + .unwrap(); + rwtxn.commit().unwrap(); + outpoint + } + + /// A transaction that spends an input without supplying an authorization + /// for it must be rejected. Otherwise the `zip` of authorizations and + /// spent UTXOs silently skips the unauthorized input, allowing any UTXO to + /// be spent without a signature. + #[test] + fn validate_transaction_rejects_missing_authorization() { + let (env, state) = new_state(); + let signing_key = SigningKey::from_bytes(&[1u8; 32]); + let verifying_key: VerifyingKey = signing_key.verifying_key().into(); + let address = authorization::get_address(&verifying_key); + let outpoint = fund(&env, &state, address, 1000); + + let transaction = Transaction::new( + vec![outpoint], + vec![Output::new( + address, + OutputContent::Bitcoin(BitcoinOutputContent( + bitcoin::Amount::from_sat(900), + )), + )], + ); + + // The attack: spend the input while providing no authorization for it. + let unauthorized = AuthorizedTransaction { + transaction: transaction.clone(), + authorizations: Vec::new(), + }; + let rotxn = env.read_txn().unwrap(); + let err = state + .validate_transaction(&rotxn, &unauthorized) + .expect_err("tx with no authorizations must be rejected"); + assert!( + matches!( + err, + Error::WrongNumberOfAuthorizations { + expected: 1, + received: 0 + } + ), + "unexpected error: {err:?}" + ); + + // The same transaction with a valid authorization is accepted. + let authorized = + authorization::authorize(&[(address, &signing_key)], transaction) + .unwrap(); + state + .validate_transaction(&rotxn, &authorized) + .expect("correctly authorized tx should validate"); + } +}