From ed024b248a9e6136a82b7abcb8aa16ae86156e9d Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Fri, 12 Jun 2026 01:28:58 +0000 Subject: [PATCH 1/6] add bitcoin-internals dependency I would like the ArrayExt trait, which has a bunch of unsafe code, and which implements stuff that is missing in stdlib. This is already in our dep tree through rust-bitcoin so adding a direct dependency adds no new code exposure. --- Cargo-recent.lock | 13 ++++++++++--- Cargo.toml | 2 ++ elementsd-tests/Cargo.toml | 1 + fuzz/Cargo.toml | 1 + 4 files changed, 14 insertions(+), 3 deletions(-) diff --git a/Cargo-recent.lock b/Cargo-recent.lock index 3deca569..5a99fb08 100644 --- a/Cargo-recent.lock +++ b/Cargo-recent.lock @@ -26,7 +26,7 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2c8d66485a3a2ea485c1913c4572ce0256067a5377ac8c75c4960e1cda98605f" dependencies = [ - "bitcoin-internals", + "bitcoin-internals 0.3.0", "bitcoin_hashes", ] @@ -66,7 +66,7 @@ dependencies = [ "base58ck", "base64 0.21.7", "bech32", - "bitcoin-internals", + "bitcoin-internals 0.3.0", "bitcoin-io", "bitcoin-units", "bitcoin_hashes", @@ -85,6 +85,12 @@ dependencies = [ "serde", ] +[[package]] +name = "bitcoin-internals" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a30a22d1f112dde8e16be7b45c63645dc165cef254f835b3e1e9553e485cfa64" + [[package]] name = "bitcoin-io" version = "0.1.3" @@ -103,7 +109,7 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5285c8bcaa25876d07f37e3d30c303f2609179716e11d688f51e8f1fe70063e2" dependencies = [ - "bitcoin-internals", + "bitcoin-internals 0.3.0", "serde", ] @@ -203,6 +209,7 @@ dependencies = [ "bech32", "bincode", "bitcoin", + "bitcoin-internals 0.5.0", "getrandom 0.2.16", "hex-conservative 1.1.0", "rand", diff --git a/Cargo.toml b/Cargo.toml index 79d9dd0b..a1d5f4af 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,6 +29,7 @@ base64 = ["bitcoin/base64"] [dependencies] bech32 = "0.11.0" bitcoin = "0.32.2" +internals = { package = "bitcoin-internals", version = "0.5" } secp256k1-zkp = { version = "0.11.0", features = ["global-context", "hashes"] } # Used for ContractHash::from_json_contract. @@ -188,6 +189,7 @@ zero_sized_map_values = "warn" [package.metadata.rbmt.lint] allowed_duplicates = [ + "bitcoin-internals", "hex-conservative", ] diff --git a/elementsd-tests/Cargo.toml b/elementsd-tests/Cargo.toml index 9523eaaa..5912f723 100644 --- a/elementsd-tests/Cargo.toml +++ b/elementsd-tests/Cargo.toml @@ -16,6 +16,7 @@ rand = "0.8" [package.metadata.rbmt.lint] # FIXME the bulk of these are because elementsd/bitcoind is much older than rust-bitcoin. allowed_duplicates = [ + "bitcoin-internals", "hex-conservative", "base64", "getrandom", diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index 3a703688..899e9a24 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -22,6 +22,7 @@ use_self = "warn" [package.metadata.rbmt.lint] allowed_duplicates = [ + "bitcoin-internals", "hex-conservative", "getrandom", "wasi", From ee50a5f36b543500d52256de9df52efa01b226ad Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Fri, 12 Jun 2026 02:30:15 +0000 Subject: [PATCH 2/6] rewrite Address::from_base58 to eliminate all the unwraps These unwraps and indexing (which hide more panic paths) irritated me, and also would need to be tweaked once we get rid of the Hash::from_slice methods. I figured I'd preemptively get rid of them. --- src/address.rs | 43 ++++++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/src/address.rs b/src/address.rs index 4a2dd2c1..4d64fd77 100644 --- a/src/address.rs +++ b/src/address.rs @@ -26,6 +26,7 @@ use crate::blech32::{Blech32, Blech32m}; use crate::hashes::Hash; use bitcoin::base58; use bitcoin::PublicKey; +use internals::array::ArrayExt as _; use secp256k1_zkp; use secp256k1_zkp::Secp256k1; use secp256k1_zkp::Verification; @@ -486,35 +487,39 @@ impl Address { // data.len() should be >= 1 when this method is called fn from_base58(data: &[u8], params: &'static AddressParams) -> Result { + let len_error = AddressError::InvalidLength(data.len()); // When unblinded, the structure is: // <1: regular prefix> <20: hash160> // When blinded, the structure is: // <1: blinding prefix> <1: regular prefix> <33: blinding pubkey> <20: hash160> - let blinded = data[0] == params.blinded_prefix; - let prefix = match (blinded, data.len()) { - (true, 55) => data[1], - (false, 21) => data[0], - (_, len) => return Err(AddressError::InvalidLength(len)), + let Some((blinding_prefix, blinded_data)) = data.split_first() else { + return Err(len_error); }; - let (blinding_pubkey, payload_data) = match blinded { - true => ( - Some( - secp256k1_zkp::PublicKey::from_slice(&data[2..35]) - .map_err(AddressError::InvalidBlindingPubKey)?, - ), - &data[35..], - ), - false => (None, &data[1..]), + let (prefix, blinding_pubkey, hash) = if *blinding_prefix == params.blinded_prefix { + let Some((prefix, pubkey_and_hash)) = blinded_data.split_first() else { + return Err(len_error); + }; + + let pubkey_and_hash = <&[u8; 53]>::try_from(pubkey_and_hash).map_err(|_| len_error)?; + let (pubkey, hash) = pubkey_and_hash.split_array::<33, 20>(); + + let blinding_pubkey = secp256k1_zkp::PublicKey::from_slice(pubkey) + .map_err(AddressError::InvalidBlindingPubKey)?; + + (prefix, Some(blinding_pubkey), hash) + } else { + let hash = <&[u8; 20]>::try_from(blinded_data).map_err(|_| len_error)?; + (blinding_prefix, None, hash) }; - let payload = if prefix == params.p2pkh_prefix { - Payload::PubkeyHash(PubkeyHash::from_slice(payload_data).unwrap()) - } else if prefix == params.p2sh_prefix { - Payload::ScriptHash(ScriptHash::from_slice(payload_data).unwrap()) + let payload = if *prefix == params.p2pkh_prefix { + Payload::PubkeyHash(PubkeyHash::from_byte_array(*hash)) + } else if *prefix == params.p2sh_prefix { + Payload::ScriptHash(ScriptHash::from_byte_array(*hash)) } else { - return Err(AddressError::InvalidAddressVersion(prefix)); + return Err(AddressError::InvalidAddressVersion(*prefix)); }; Ok(Address { From 6809a7f27de2f94e2f1d660149c9fa5f50670308 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 15 Jun 2026 15:15:24 +0000 Subject: [PATCH 3/6] blind: use array splitting in TxOut::unblind (fix potential DoS?) I don't *think* it's possible to create a rangeproof with a sidechannel smaller than 64 bytes (if you create a 0-sized "proof of exact value" then unwinding will fail entirely, and anything larger I think has at least one ring, so 128 bytes or more). Unsure. But better not to assume this by indexing recklessly into the sidechannel message. --- src/blind.rs | 28 ++++++++++++++++++++++------ src/confidential.rs | 5 +++++ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/blind.rs b/src/blind.rs index 1d31a36c..7954ddcc 100644 --- a/src/blind.rs +++ b/src/blind.rs @@ -15,7 +15,8 @@ //! # Transactions Blinding //! -use core::convert::TryFrom; +use internals::array::ArrayExt as _; +use internals::slice::SliceExt; use std::{self, collections::BTreeMap, fmt}; use secp256k1_zkp::{ @@ -761,16 +762,31 @@ impl TxOut { additional_generator, )?; - let (asset, asset_bf) = opening.message.as_ref().split_at(32); - let asset = <[u8; 32]>::try_from(asset).map_err(UnblindError::MalformedAssetId)?; - let asset = AssetId::from_byte_array(asset); - let asset_bf = AssetBlindingFactor::from_slice(&asset_bf[..32])?; + // Use `MissingRangeproof` error because it's available so does not require + // API breaks. In a later PR we should extend that enum and add #[non_exhaustive] + // to it. The maybe-better `MalformedAssetId` error requires we start with a + // std `FromSliceError` which we don't have. + let asset_and_bf = SliceExt::split_first_chunk::<64>(opening.message.as_ref()) + .ok_or(UnblindError::MissingRangeproof)? + .0; + let (asset_id, asset_bf) = asset_and_bf.split_array(); + + let asset_id = AssetId::from_byte_array(*asset_id); + let asset_bf = AssetBlindingFactor::from_byte_array(*asset_bf)?; + if let Asset::Confidential(own_asset) = self.asset { + let secp = Secp256k1::signing_only(); // needed to avoid API break + let asset = Generator::new_blinded(&secp, asset_id.into_tag(), asset_bf.into_inner()); + if asset != own_asset { + // See above about use of MissingRangeproof. + return Err(UnblindError::MissingRangeproof); + } + } let value = opening.value; let value_bf = ValueBlindingFactor(opening.blinding_factor); Ok(TxOutSecrets { - asset, + asset: asset_id, asset_bf, value, value_bf, diff --git a/src/confidential.rs b/src/confidential.rs index fa0e6dca..ad3066b0 100644 --- a/src/confidential.rs +++ b/src/confidential.rs @@ -789,6 +789,11 @@ impl AssetBlindingFactor { s.parse() } + /// Create from bytes. + pub fn from_byte_array(bytes: [u8; 32]) -> Result { + Ok(AssetBlindingFactor(Tweak::from_inner(bytes)?)) + } + /// Create from bytes. pub fn from_slice(bytes: &[u8]) -> Result { Ok(AssetBlindingFactor(Tweak::from_slice(bytes)?)) From cae359d0c14deba426ce24e038f8f9f46da1ea5c Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 15 Jun 2026 16:27:30 +0000 Subject: [PATCH 4/6] transaction: use better error typing for pegin destructuring We should redo the error types here as well at some point. --- src/transaction.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/transaction.rs b/src/transaction.rs index 7443c41e..367174b7 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -20,6 +20,7 @@ use std::collections::HashMap; use std::convert::TryFrom; use bitcoin::{self, VarInt}; +use internals::slice::SliceExt; use crate::hashes::Hash; use crate::{confidential, ContractHash}; @@ -432,12 +433,12 @@ impl<'tx> PeginData<'tx> { pegin_witness: &'tx [Vec], prevout: bitcoin::OutPoint, ) -> Result, &'static str> { - if pegin_witness.len() != 6 { + let Ok(pegin_witness) = <&[Vec; 6]>::try_from(pegin_witness) else { return Err("size not 6"); - } - if pegin_witness[5].len() < 80 { + }; + let Some((block_header, _)) = SliceExt::split_first_chunk::<80>(pegin_witness[5].as_slice()) else { return Err("merkle proof too short"); - } + }; Ok(PeginData { outpoint: prevout, @@ -448,7 +449,7 @@ impl<'tx> PeginData<'tx> { claim_script: &pegin_witness[3], tx: &pegin_witness[4], merkle_proof: &pegin_witness[5], - referenced_block: bitcoin::BlockHash::hash(&pegin_witness[5][0..80]), + referenced_block: bitcoin::BlockHash::hash(block_header), }) } From 43840124da672e989235db338bfe7e577062f285 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 15 Jun 2026 18:06:13 +0000 Subject: [PATCH 5/6] address: do a better job slicing bech32 data Currently we allow decoding segwit v0 programs which have uncompressed/hybrid keys (not allowed) and I suspect that if you provide a too-short address then you'll get a panic here. --- src/address.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/address.rs b/src/address.rs index 4d64fd77..7480ccf9 100644 --- a/src/address.rs +++ b/src/address.rs @@ -27,6 +27,7 @@ use crate::hashes::Hash; use bitcoin::base58; use bitcoin::PublicKey; use internals::array::ArrayExt as _; +use internals::slice::SliceExt; use secp256k1_zkp; use secp256k1_zkp::Secp256k1; use secp256k1_zkp::Verification; @@ -468,13 +469,17 @@ impl Address { }; let (blinding_pubkey, program) = match blinded { - true => ( + true => { + let (pk, rest) = SliceExt::split_first_chunk::<33>(data.as_slice()) + .ok_or(AddressError::InvalidSegwitV0Encoding)?; + ( Some( - secp256k1_zkp::PublicKey::from_slice(&data[..33]) + secp256k1_zkp::PublicKey::from_slice(pk) .map_err(AddressError::InvalidBlindingPubKey)?, - ), - data[33..].to_vec(), - ), + ), + rest.to_vec(), + ) + }, false => (None, data), }; From 5e0d5774d96313e3d65dc47041cedc907a40b8de Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 15 Jun 2026 18:35:30 +0000 Subject: [PATCH 6/6] pset: fix slicing in KeySource::deserialize Lol, this code was so close and yet so far. --- src/pset/serialize.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/pset/serialize.rs b/src/pset/serialize.rs index 227e21f5..5b152c04 100644 --- a/src/pset/serialize.rs +++ b/src/pset/serialize.rs @@ -17,7 +17,6 @@ //! Defines traits used for (de)serializing PSET values into/from raw //! bytes in PSET key-value pairs. -use std::convert::TryFrom; use std::io; use crate::confidential::{self, AssetBlindingFactor}; @@ -29,6 +28,7 @@ use crate::{AssetId, BlockHash, Script, Transaction, TxOut, Txid}; use bitcoin; use bitcoin::bip32::{ChildNumber, Fingerprint, KeySource}; use bitcoin::{key::XOnlyPublicKey, PublicKey}; +use internals::slice::SliceExt; use secp256k1_zkp::{self, RangeProof, SurjectionProof, Tweak}; use super::map::{PsbtSighashType, TapTree}; @@ -176,16 +176,15 @@ impl Serialize for KeySource { impl Deserialize for KeySource { fn deserialize(bytes: &[u8]) -> Result { - let Ok(prefix) = <[u8; 4]>::try_from(&bytes[0..4]) else { + let Some((prefix, mut rest)) = SliceExt::split_first_chunk::<4>(bytes) else { return Err(io::Error::from(io::ErrorKind::UnexpectedEof).into()); }; let fprint: Fingerprint = Fingerprint::from(prefix); let mut dpath: Vec = Vec::default(); - let mut d = &bytes[4..]; - while !d.is_empty() { - let index = u32::consensus_decode(&mut d)?; + while !rest.is_empty() { + let index = u32::consensus_decode(&mut rest)?; dpath.push(index.into()); }