diff --git a/src/confidential/asset.rs b/src/confidential/asset.rs index 5636b4bf..a8a24ff1 100644 --- a/src/confidential/asset.rs +++ b/src/confidential/asset.rs @@ -10,7 +10,7 @@ use secp256k1_zkp::{self, Generator, Secp256k1, Signing, Tweak, ZERO_TWEAK}; #[cfg(feature = "serde")] use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use super::CommitmentEncoder; +use super::{checked_commitment_slice, CommitmentEncoder, CONFIDENTIAL_LEN}; use crate::encode::{self, Decodable, Encodable}; use crate::encoding; use crate::issuance::AssetId; @@ -19,7 +19,6 @@ type ExplicitInner = AssetId; type ConfInner = Generator; const EXPLICIT_LEN: usize = 32; -const CONFIDENTIAL_LEN: usize = 33; const CONFIDENTIAL_LEN_LESS_PREFIX: usize = CONFIDENTIAL_LEN - 1; const CONF_PREFIX_1: u8 = 0x0a; const CONF_PREFIX_2: u8 = 0x0b; @@ -57,7 +56,7 @@ impl Asset { /// Create from commitment. pub fn from_commitment(bytes: &[u8]) -> Result { - Ok(Self::Confidential(ConfInner::from_slice(bytes)?)) + Ok(Self::Confidential(ConfInner::from_slice(checked_commitment_slice(bytes)?)?)) } /// Check if the object is null. diff --git a/src/confidential/mod.rs b/src/confidential/mod.rs index 22fe9542..bfbdac99 100644 --- a/src/confidential/mod.rs +++ b/src/confidential/mod.rs @@ -51,6 +51,16 @@ pub use self::value::{ use crate::issuance::AssetId; use crate::{encode, encoding}; +const CONFIDENTIAL_LEN: usize = 33; + +pub(crate) fn checked_commitment_slice(bytes: &[u8]) -> Result<&[u8], encode::Error> { + // The upstream FFI parsers take no length and unconditionally read 33 bytes. + if bytes.len() != CONFIDENTIAL_LEN { + return Err(encode::Error::ParseFailed("invalid confidential commitment length")); + } + Ok(bytes) +} + #[derive(Clone, Debug)] enum CommitmentEncoder<'e> { Null(u8), @@ -323,6 +333,13 @@ mod tests { #[test] fn commitments() { + for len in [0usize, 1, 32, 34] { + let bytes = vec![0u8; len]; + assert!(Value::from_commitment(&bytes).is_err()); + assert!(Asset::from_commitment(&bytes).is_err()); + assert!(Nonce::from_commitment(&bytes).is_err()); + } + let x = Value::from_commitment(&VALUE_COMMITMENT1).unwrap(); let commitment = x.commitment().unwrap(); let mut commitment = commitment.serialize(); diff --git a/src/confidential/nonce.rs b/src/confidential/nonce.rs index 12310e0c..49185a65 100644 --- a/src/confidential/nonce.rs +++ b/src/confidential/nonce.rs @@ -10,7 +10,7 @@ use secp256k1_zkp::{self, PublicKey, Secp256k1, SecretKey, Signing}; #[cfg(feature = "serde")] use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use super::CommitmentEncoder; +use super::{checked_commitment_slice, CommitmentEncoder, CONFIDENTIAL_LEN}; use crate::encode::{self, Decodable, Encodable}; use crate::encoding; use crate::hashes::sha256d; @@ -19,7 +19,6 @@ type ExplicitInner = [u8; 32]; type ConfInner = PublicKey; const EXPLICIT_LEN: usize = 32; -const CONFIDENTIAL_LEN: usize = 33; const CONFIDENTIAL_LEN_LESS_PREFIX: usize = CONFIDENTIAL_LEN - 1; const CONF_PREFIX_1: u8 = 0x02; const CONF_PREFIX_2: u8 = 0x03; @@ -99,7 +98,8 @@ impl Nonce { /// Create from commitment. pub fn from_commitment(bytes: &[u8]) -> Result { Ok(Self::Confidential( - ConfInner::from_slice(bytes).map_err(secp256k1_zkp::Error::Upstream)?, + ConfInner::from_slice(checked_commitment_slice(bytes)?) + .map_err(secp256k1_zkp::Error::Upstream)?, )) } diff --git a/src/confidential/value.rs b/src/confidential/value.rs index 930e20c3..c992c821 100644 --- a/src/confidential/value.rs +++ b/src/confidential/value.rs @@ -14,7 +14,7 @@ use secp256k1_zkp::{ #[cfg(feature = "serde")] use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use super::CommitmentEncoder; +use super::{checked_commitment_slice, CommitmentEncoder, CONFIDENTIAL_LEN}; use crate::confidential::AssetBlindingFactor; use crate::encode::{self, Decodable, Encodable}; use crate::encoding; @@ -24,7 +24,6 @@ type ExplicitInner = u64; type ConfInner = PedersenCommitment; const EXPLICIT_LEN: usize = 8; -const CONFIDENTIAL_LEN: usize = 33; const CONFIDENTIAL_LEN_LESS_PREFIX: usize = CONFIDENTIAL_LEN - 1; const CONF_PREFIX_1: u8 = 0x08; const CONF_PREFIX_2: u8 = 0x09; @@ -78,7 +77,7 @@ impl Value { /// Create from commitment. pub fn from_commitment(bytes: &[u8]) -> Result { - Ok(Self::Confidential(ConfInner::from_slice(bytes)?)) + Ok(Self::Confidential(ConfInner::from_slice(checked_commitment_slice(bytes)?)?)) } /// Check if the object is null. diff --git a/src/pset/mod.rs b/src/pset/mod.rs index d4f254be..d8861477 100644 --- a/src/pset/mod.rs +++ b/src/pset/mod.rs @@ -807,6 +807,32 @@ mod tests { assert_eq!(encode::serialize_hex(&pset), pset_hex); } + #[test] + fn deserialize_commitments_rejects_bad_lengths() { + use crate::pset::serialize::Deserialize as _; + + // The upstream FFI parsers read 33 bytes without a length argument; + // empty and short inputs must be rejected before reaching FFI. + for len in [0usize, 1, 32, 34] { + let bytes = vec![0u8; len]; + assert!(secp256k1_zkp::PedersenCommitment::deserialize(&bytes).is_err()); + assert!(secp256k1_zkp::Generator::deserialize(&bytes).is_err()); + } + // A 33-byte input reaches FFI and is rejected on content, not by crash. + let bytes = vec![0u8; 33]; + assert!(secp256k1_zkp::PedersenCommitment::deserialize(&bytes).is_err()); + assert!(secp256k1_zkp::Generator::deserialize(&bytes).is_err()); + } + + #[test] + fn deserialize_malformed_pset_returns_error() { + // Minimized libFuzzer artifact (397 bytes): deserializing this input + // segfaulted (read at address 0x1) instead of returning an error. + let bytes = hex::decode_to_vec("70736574ff01050a02ffffc50070736574ff010204ccbeff0001fb040200000027ff030000feff010000000500005808080032000081040808080808080808080808080808080f080808736574ff01fb0104ff0001fb040200000027ff030000feff0100000005000058080808080808080808080808080808080072700008736574ff01fb0104010000d670736574ff000005000070736574ff030000020001000005002929220a0a0202027073654545454545454547474747474747471111111100ff1111111111116574d0ffffff011107fc04707365740100001111111111111111111111111111010074ff0a0a02b102020229292929292929220a0a0202027073654545454545454574ff02b80202020a0a454545454545454545ffe2e2e20002006511111111111111111111040200000027ff030000feff0100ffffffffffffffffffff080808080808080808080f0808087347474747474747471111111100ff1111111111116574d0ffffff011107fc047073657401000011110100000000000000111111110100").unwrap(); + let result: Result = encode::deserialize(&bytes); + assert!(result.is_err()); + } + #[test] fn test_pset() { tx_pset_rtt("010000000001715df5ccebaf02ff18d6fae7263fa69fed5de59c900f4749556eba41bc7bf2af0000000000000000000201230f4f5d4b7c6fa845806ee4f67713459e1b69e8e60fcee2e4940c7a0d5de1b2010000000124101100001f5175517551755175517551755175517551755175517551755175517551755101230f4f5d4b7c6fa845806ee4f67713459e1b69e8e60fcee2e4940c7a0d5de1b2010000000005f5e100000000000000"); diff --git a/src/pset/serialize.rs b/src/pset/serialize.rs index cf39eaca..8523556c 100644 --- a/src/pset/serialize.rs +++ b/src/pset/serialize.rs @@ -235,7 +235,9 @@ impl Serialize for secp256k1_zkp::PedersenCommitment { impl Deserialize for secp256k1_zkp::PedersenCommitment { fn deserialize(bytes: &[u8]) -> Result { - let comm = secp256k1_zkp::PedersenCommitment::from_slice(bytes)?; + let comm = secp256k1_zkp::PedersenCommitment::from_slice( + confidential::checked_commitment_slice(bytes)?, + )?; Ok(comm) } } @@ -248,7 +250,9 @@ impl Serialize for secp256k1_zkp::Generator { impl Deserialize for secp256k1_zkp::Generator { fn deserialize(bytes: &[u8]) -> Result { - let comm = secp256k1_zkp::Generator::from_slice(bytes)?; + let comm = secp256k1_zkp::Generator::from_slice( + confidential::checked_commitment_slice(bytes)?, + )?; Ok(comm) } }