fix(platform)!: 3.0 audit report fixes

packages/rs-dpp/src/address_funds/fee_strategy/deduct_fee_from_inputs_and_outputs/v0/mod.rs

@@ -24,37 +24,46 @@ pub fn deduct_fee_from_outputs_or_remaining_balance_of_inputs_v0(
 ) -> Result<FeeDeductionResult, ProtocolError> {
     let mut remaining_fee = fee;
 
+    // Snapshot addresses before any mutations so indices remain stable.
+    // Without this, removing a drained entry shifts all subsequent indices.
+    let input_addresses: Vec<PlatformAddress> = inputs.keys().copied().collect();
+    let output_addresses: Vec<PlatformAddress> = outputs.keys().copied().collect();
+
     for step in fee_strategy {
         if remaining_fee == 0 {
             break;
         }
 
         match step {
             AddressFundsFeeStrategyStep::DeductFromInput(index) => {
-                // Reduce the remaining balance of the input at the specified index
-                if let Some((&address, &(nonce, amount))) = inputs.iter().nth(*index as usize) {
-                    let reduction = remaining_fee.min(amount);
-                    let new_amount = amount - reduction;
-                    remaining_fee -= reduction;
+                // Resolve the index via the original snapshot, then look up by key
+                if let Some(&address) = input_addresses.get(*index as usize) {
+                    if let Some(&(nonce, amount)) = inputs.get(&address) {
+                        let reduction = remaining_fee.min(amount);
+                        let new_amount = amount - reduction;
+                        remaining_fee -= reduction;
 
-                    if new_amount == 0 {
-                        inputs.remove(&address);
-                    } else {
-                        inputs.insert(address, (nonce, new_amount));
+                        if new_amount == 0 {
+                            inputs.remove(&address);
+                        } else {
+                            inputs.insert(address, (nonce, new_amount));
+                        }
                     }
                 }
             }
             AddressFundsFeeStrategyStep::ReduceOutput(index) => {
-                // Reduce the output at the specified index
-                if let Some((&address, &amount)) = outputs.iter().nth(*index as usize) {
-                    let reduction = remaining_fee.min(amount);
-                    let new_amount = amount - reduction;
-                    remaining_fee -= reduction;
+                // Resolve the index via the original snapshot, then look up by key
+                if let Some(&address) = output_addresses.get(*index as usize) {
+                    if let Some(&amount) = outputs.get(&address) {
+                        let reduction = remaining_fee.min(amount);
+                        let new_amount = amount - reduction;
+                        remaining_fee -= reduction;
 
-                    if new_amount == 0 {
-                        outputs.remove(&address);
-                    } else {
-                        outputs.insert(address, new_amount);
+                        if new_amount == 0 {
+                            outputs.remove(&address);
+                        } else {
+                            outputs.insert(address, new_amount);
+                        }
                     }
                 }
             }

packages/rs-dpp/src/address_funds/witness.rs

@@ -6,6 +6,10 @@ use platform_value::BinaryData;
 #[cfg(feature = "state-transition-serde-conversion")]
 use serde::{Deserialize, Serialize};
 
+/// Maximum number of entries in a P2SH signatures vector.
+/// This is 16 (max keys from OP_PUSHNUM_16) + 1 (CHECKMULTISIG dummy byte).
+pub const MAX_P2SH_SIGNATURES: usize = 17;
+
 /// The input witness data required to spend from a PlatformAddress.
 ///
 /// This enum captures the different spending patterns for P2PKH and P2SH addresses.
@@ -63,6 +67,13 @@ impl<C> Decode<C> for AddressWitness {
             }
             1 => {
                 let signatures = Vec::<BinaryData>::decode(decoder)?;
+                if signatures.len() > MAX_P2SH_SIGNATURES {
+                    return Err(DecodeError::OtherString(format!(
+                        "P2SH signatures count {} exceeds maximum {}",
+                        signatures.len(),
+                        MAX_P2SH_SIGNATURES,
+                    )));
+                }
                 let redeem_script = BinaryData::decode(decoder)?;
                 Ok(AddressWitness::P2sh {
                     signatures,
@@ -89,6 +100,13 @@ impl<'de, C> bincode::BorrowDecode<'de, C> for AddressWitness {
             }
             1 => {
                 let signatures = Vec::<BinaryData>::borrow_decode(decoder)?;
+                if signatures.len() > MAX_P2SH_SIGNATURES {
+                    return Err(DecodeError::OtherString(format!(
+                        "P2SH signatures count {} exceeds maximum {}",
+                        signatures.len(),
+                        MAX_P2SH_SIGNATURES,
+                    )));
+                }
                 let redeem_script = BinaryData::borrow_decode(decoder)?;
                 Ok(AddressWitness::P2sh {
                     signatures,
@@ -190,6 +208,13 @@ impl<'de> Deserialize<'de> for AddressWitness {
                     "p2sh" => {
                         let signatures =
                             signatures.ok_or_else(|| de::Error::missing_field("signatures"))?;
+                        if signatures.len() > MAX_P2SH_SIGNATURES {
+                            return Err(de::Error::custom(format!(
+                                "P2SH signatures count {} exceeds maximum {}",
+                                signatures.len(),
+                                MAX_P2SH_SIGNATURES,
+                            )));
+                        }
                         let redeem_script = redeem_script
                             .ok_or_else(|| de::Error::missing_field("redeemScript"))?;
                         Ok(AddressWitness::P2sh {
@@ -380,4 +405,109 @@ mod tests {
 
         assert_eq!(witness, deserialized);
     }
+
+    /// AUDIT L1: Unbounded P2SH witness size during deserialization.
+    ///
+    /// The `Decode` impl for `AddressWitness::P2sh` now enforces
+    /// `MAX_P2SH_SIGNATURES` during deserialization. A payload with more
+    /// signatures than the limit is rejected with a decode error.
+    ///
+    /// Location: rs-dpp/src/address_funds/witness.rs
+    #[test]
+    fn test_p2sh_witness_rejects_excessive_signatures() {
+        // Create a P2SH witness with 1000 signatures — far above MAX_P2SH_SIGNATURES
+        let num_signatures = 1000;
+        let signatures: Vec<BinaryData> = (0..num_signatures)
+            .map(|i| BinaryData::new(vec![0x30, 0x44, i as u8]))
+            .collect();
+
+        let witness = AddressWitness::P2sh {
+            signatures,
+            redeem_script: BinaryData::new(vec![0x52, 0xae]),
+        };
+
+        // Encode succeeds (encoding has no limit), but decode must reject
+        let encoded = bincode::encode_to_vec(&witness, config::standard()).unwrap();
+        let result: Result<(AddressWitness, usize), _> =
+            bincode::decode_from_slice(&encoded, config::standard());
+
+        assert!(
+            result.is_err(),
+            "AUDIT L1: P2SH witness with {} signatures should be rejected during \
+            deserialization. MAX_P2SH_SIGNATURES = {}.",
+            num_signatures,
+            MAX_P2SH_SIGNATURES,
+        );
+    }
+
+    /// AUDIT L3: No maximum length check on P2SH signatures vector.
+    ///
+    /// The deserialization now enforces `MAX_P2SH_SIGNATURES` (17). Signature
+    /// counts above this limit are rejected during decode. The boundary value
+    /// (17) is accepted, and 18+ is rejected.
+    ///
+    /// Location: rs-dpp/src/address_funds/witness.rs
+    #[test]
+    fn test_p2sh_witness_max_signatures_boundary() {
+        // Counts above MAX_P2SH_SIGNATURES should be rejected during decode
+        for count in [50, 100, 500] {
+            let signatures: Vec<BinaryData> = (0..count)
+                .map(|_| BinaryData::new(vec![0x30, 0x44, 0x02, 0x20]))
+                .collect();
+
+            let witness = AddressWitness::P2sh {
+                signatures,
+                redeem_script: BinaryData::new(vec![0x52, 0xae]),
+            };
+
+            let encoded = bincode::encode_to_vec(&witness, config::standard()).unwrap();
+            let result: Result<(AddressWitness, usize), _> =
+                bincode::decode_from_slice(&encoded, config::standard());
+
+            assert!(
+                result.is_err(),
+                "AUDIT L3: P2SH witness with {} signatures should be rejected during \
+                deserialization. MAX_P2SH_SIGNATURES = {}.",
+                count,
+                MAX_P2SH_SIGNATURES,
+            );
+        }
+
+        // MAX_P2SH_SIGNATURES (17) should be accepted
+        let signatures: Vec<BinaryData> = (0..MAX_P2SH_SIGNATURES)
+            .map(|_| BinaryData::new(vec![0x30, 0x44, 0x02, 0x20]))
+            .collect();
+
+        let witness = AddressWitness::P2sh {
+            signatures,
+            redeem_script: BinaryData::new(vec![0x52, 0xae]),
+        };
+
+        let encoded = bincode::encode_to_vec(&witness, config::standard()).unwrap();
+        let decoded: AddressWitness = bincode::decode_from_slice(&encoded, config::standard())
+            .unwrap()
+            .0;
+
+        assert_eq!(witness, decoded);
+
+        // MAX_P2SH_SIGNATURES + 1 should be rejected
+        let signatures: Vec<BinaryData> = (0..MAX_P2SH_SIGNATURES + 1)
+            .map(|_| BinaryData::new(vec![0x30, 0x44, 0x02, 0x20]))
+            .collect();
+
+        let witness = AddressWitness::P2sh {
+            signatures,
+            redeem_script: BinaryData::new(vec![0x52, 0xae]),
+        };
+
+        let encoded = bincode::encode_to_vec(&witness, config::standard()).unwrap();
+        let result: Result<(AddressWitness, usize), _> =
+            bincode::decode_from_slice(&encoded, config::standard());
+
+        assert!(
+            result.is_err(),
+            "P2SH witness with {} signatures (MAX + 1) should be rejected",
+            MAX_P2SH_SIGNATURES + 1,
+        );
+    }
 }

packages/rs-dpp/src/errors/consensus/basic/basic_error.rs

@@ -80,6 +80,7 @@ use crate::consensus::basic::state_transition::{
     OutputsNotGreaterThanInputsError, StateTransitionMaxSizeExceededError,
     StateTransitionNotActiveError, TransitionNoInputsError, TransitionNoOutputsError,
     TransitionOverMaxInputsError, TransitionOverMaxOutputsError, WithdrawalBalanceMismatchError,
+    WithdrawalBelowMinAmountError,
 };
 use crate::consensus::basic::{
     IncompatibleProtocolVersionError, UnsupportedFeatureError, UnsupportedProtocolVersionError,
@@ -645,6 +646,9 @@ pub enum BasicError {
     #[error(transparent)]
     WithdrawalBalanceMismatchError(WithdrawalBalanceMismatchError),
 
+    #[error(transparent)]
+    WithdrawalBelowMinAmountError(WithdrawalBelowMinAmountError),
+
     #[error(transparent)]
     InsufficientFundingAmountError(InsufficientFundingAmountError),
 

packages/rs-dpp/src/errors/consensus/basic/state_transition/mod.rs

@@ -20,6 +20,7 @@ mod transition_no_outputs_error;
 mod transition_over_max_inputs_error;
 mod transition_over_max_outputs_error;
 mod withdrawal_balance_mismatch_error;
+mod withdrawal_below_min_amount_error;
 
 pub use fee_strategy_duplicate_error::*;
 pub use fee_strategy_empty_error::*;
@@ -43,3 +44,4 @@ pub use transition_no_outputs_error::*;
 pub use transition_over_max_inputs_error::*;
 pub use transition_over_max_outputs_error::*;
 pub use withdrawal_balance_mismatch_error::*;
+pub use withdrawal_below_min_amount_error::*;

packages/rs-dpp/src/errors/consensus/basic/state_transition/withdrawal_below_min_amount_error.rs

@@ -0,0 +1,52 @@
+use crate::consensus::basic::BasicError;
+use crate::consensus::ConsensusError;
+use crate::errors::ProtocolError;
+use bincode::{Decode, Encode};
+use platform_serialization_derive::{PlatformDeserialize, PlatformSerialize};
+use thiserror::Error;
+
+#[derive(
+    Error, Debug, Clone, PartialEq, Eq, Encode, Decode, PlatformSerialize, PlatformDeserialize,
+)]
+#[error(
+    "Withdrawal amount {withdrawal_amount} must be at least {min_amount} and at most {max_amount}"
+)]
+#[platform_serialize(unversioned)]
+pub struct WithdrawalBelowMinAmountError {
+    /*
+
+    DO NOT CHANGE ORDER OF FIELDS WITHOUT INTRODUCING OF NEW VERSION
+
+    */
+    withdrawal_amount: u64,
+    min_amount: u64,
+    max_amount: u64,
+}
+
+impl WithdrawalBelowMinAmountError {
+    pub fn new(withdrawal_amount: u64, min_amount: u64, max_amount: u64) -> Self {
+        Self {
+            withdrawal_amount,
+            min_amount,
+            max_amount,
+        }
+    }
+
+    pub fn withdrawal_amount(&self) -> u64 {
+        self.withdrawal_amount
+    }
+
+    pub fn min_amount(&self) -> u64 {
+        self.min_amount
+    }
+
+    pub fn max_amount(&self) -> u64 {
+        self.max_amount
+    }
+}
+
+impl From<WithdrawalBelowMinAmountError> for ConsensusError {
+    fn from(err: WithdrawalBelowMinAmountError) -> Self {
+        Self::BasicError(BasicError::WithdrawalBelowMinAmountError(err))
+    }
+}

packages/rs-dpp/src/errors/consensus/codes.rs

@@ -231,6 +231,7 @@ impl ErrorWithCode for BasicError {
             Self::InputsNotLessThanOutputsError(_) => 10815,
             Self::OutputAddressAlsoInputError(_) => 10816,
             Self::InvalidRemainderOutputCountError(_) => 10817,
+            Self::WithdrawalBelowMinAmountError(_) => 10818,
         }
     }
 }