pset: check commitment lengths before FFI parsing

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<Self, encode::Error> {
-        Ok(Self::Confidential(ConfInner::from_slice(bytes)?))
+        Ok(Self::Confidential(ConfInner::from_slice(checked_commitment_slice(bytes)?)?))
     }
 
     /// Check if the object is null.

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();

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<Self, encode::Error> {
         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)?,
         ))
     }
 

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<Self, encode::Error> {
-        Ok(Self::Confidential(ConfInner::from_slice(bytes)?))
+        Ok(Self::Confidential(ConfInner::from_slice(checked_commitment_slice(bytes)?)?))
     }
 
     /// Check if the object is null.

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<PartiallySignedTransaction, _> = encode::deserialize(&bytes);
+        assert!(result.is_err());
+    }
+
     #[test]
     fn test_pset() {
         tx_pset_rtt("010000000001715df5ccebaf02ff18d6fae7263fa69fed5de59c900f4749556eba41bc7bf2af0000000000000000000201230f4f5d4b7c6fa845806ee4f67713459e1b69e8e60fcee2e4940c7a0d5de1b2010000000124101100001f5175517551755175517551755175517551755175517551755175517551755101230f4f5d4b7c6fa845806ee4f67713459e1b69e8e60fcee2e4940c7a0d5de1b2010000000005f5e100000000000000");

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<Self, encode::Error> {
-        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<Self, encode::Error> {
-        let comm = secp256k1_zkp::Generator::from_slice(bytes)?;
+        let comm = secp256k1_zkp::Generator::from_slice(
+            confidential::checked_commitment_slice(bytes)?,
+        )?;
         Ok(comm)
     }
 }