fix(platform)!: refuse high-S signatures on key-recovery paths from protocol version 15

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

@@ -12,12 +12,11 @@ use bincode::{Decode, DecodeUntrusted, Encode};
 use dashcore::address::Payload;
 use dashcore::blockdata::script::ScriptBuf;
 use dashcore::hashes::sha256d;
-use dashcore::{
-    sign_message, signer, Address, Network, PrivateKey, PubkeyHash, PublicKey, ScriptHash,
-};
+use dashcore::{sign_message, Address, Network, PrivateKey, PubkeyHash, PublicKey, ScriptHash};
 use platform_serialization_derive::{
     PlatformDeserializeTrusted, PlatformDeserializeUntrusted, PlatformSerialize,
 };
+use platform_version::version::PlatformVersion;
 #[cfg(feature = "serde-conversion")]
 use serde::{Deserialize, Serialize};
 use std::convert::TryFrom;
@@ -473,14 +472,19 @@ impl PlatformAddress {
     /// # Arguments
     /// * `witness` - The witness containing signature(s) and either a public key (P2PKH) or redeem script (P2SH)
     /// * `signable_bytes` - The data that was signed (will be double-SHA256 hashed internally)
+    /// * `platform_version` - Selects `verify_hash_signature` for a P2PKH witness: from protocol
+    ///   version 15 a high-S signature and a header naming an uncompressed key are refused.
     ///
     /// # Returns
     /// * `Ok(AddressWitnessVerificationOperations)` - Operations performed if verification succeeds
-    /// * `Err(ProtocolError)` if verification fails
+    /// * `Err(ProtocolError::UnknownVersionMismatch)` if this build does not know the selected
+    ///   `verify_hash_signature` version
+    /// * `Err(ProtocolError)` of another kind if verification fails
     pub fn verify_bytes_against_witness(
         &self,
         witness: &AddressWitness,
         signable_bytes: &[u8],
+        platform_version: &PlatformVersion,
     ) -> Result<AddressWitnessVerificationOperations, ProtocolError> {
         match (self, witness) {
             (PlatformAddress::P2pkh(pubkey_hash), AddressWitness::P2pkh { signature }) => {
@@ -492,13 +496,18 @@ impl PlatformAddress {
                 // This saves 33 bytes per witness (no need to include pubkey)
                 // at a ~4% CPU cost increase (recovery vs verify).
                 let data_hash = dashcore::signer::double_sha(signable_bytes);
-                signer::verify_hash_signature(&data_hash, signature.as_slice(), pubkey_hash)
-                    .map_err(|e| {
-                        ProtocolError::AddressWitnessError(format!(
-                            "P2PKH signature verification failed: {}",
-                            e
-                        ))
-                    })?;
+                ecdsa::verify_hash_signature(
+                    &data_hash,
+                    signature.as_slice(),
+                    pubkey_hash,
+                    platform_version,
+                )?
+                .map_err(|e| {
+                    ProtocolError::AddressWitnessError(format!(
+                        "P2PKH signature verification failed: {}",
+                        e
+                    ))
+                })?;
 
                 Ok(AddressWitnessVerificationOperations::for_p2pkh(
                     signable_bytes.len(),
@@ -896,7 +905,11 @@ mod tests {
         };
 
         // Verify should succeed
-        let result = address.verify_bytes_against_witness(&witness, signable_bytes);
+        let result = address.verify_bytes_against_witness(
+            &witness,
+            signable_bytes,
+            PlatformVersion::latest(),
+        );
         assert!(
             result.is_ok(),
             "P2PKH verification should succeed: {:?}",
@@ -925,7 +938,8 @@ mod tests {
         };
 
         // Verify should fail (recovered pubkey won't match because message differs)
-        let result = address.verify_bytes_against_witness(&witness, verify_bytes);
+        let result =
+            address.verify_bytes_against_witness(&witness, verify_bytes, PlatformVersion::latest());
         assert!(
             result.is_err(),
             "P2PKH verification should fail with wrong data"
@@ -954,7 +968,11 @@ mod tests {
         };
 
         // Verify should fail (recovered pubkey hash won't match address)
-        let result = address.verify_bytes_against_witness(&witness, signable_bytes);
+        let result = address.verify_bytes_against_witness(
+            &witness,
+            signable_bytes,
+            PlatformVersion::latest(),
+        );
         assert!(
             result.is_err(),
             "P2PKH verification should fail when signed with wrong key"
@@ -995,7 +1013,11 @@ mod tests {
         };
 
         // Verify should succeed
-        let result = address.verify_bytes_against_witness(&witness, signable_bytes);
+        let result = address.verify_bytes_against_witness(
+            &witness,
+            signable_bytes,
+            PlatformVersion::latest(),
+        );
         assert!(
             result.is_ok(),
             "P2SH 2-of-3 multisig verification should succeed: {:?}",
@@ -1032,7 +1054,11 @@ mod tests {
         };
 
         // Verify should succeed
-        let result = address.verify_bytes_against_witness(&witness, signable_bytes);
+        let result = address.verify_bytes_against_witness(
+            &witness,
+            signable_bytes,
+            PlatformVersion::latest(),
+        );
         assert!(
             result.is_ok(),
             "P2SH 2-of-3 multisig with keys 1 and 2 should succeed: {:?}",
@@ -1068,7 +1094,11 @@ mod tests {
         };
 
         // Verify should fail
-        let result = address.verify_bytes_against_witness(&witness, signable_bytes);
+        let result = address.verify_bytes_against_witness(
+            &witness,
+            signable_bytes,
+            PlatformVersion::latest(),
+        );
         assert!(
             result.is_err(),
             "P2SH should fail with only 1 signature when 2 required"
@@ -1107,7 +1137,11 @@ mod tests {
         };
 
         // Verify should fail (script doesn't hash to address)
-        let result = address.verify_bytes_against_witness(&witness, signable_bytes);
+        let result = address.verify_bytes_against_witness(
+            &witness,
+            signable_bytes,
+            PlatformVersion::latest(),
+        );
         assert!(
             result.is_err(),
             "P2SH should fail when script hash doesn't match address"
@@ -1148,8 +1182,11 @@ mod tests {
         let p2pkh_witness = AddressWitness::P2pkh {
             signature: BinaryData::new(p2pkh_sig),
         };
-        let p2pkh_result =
-            p2pkh_address.verify_bytes_against_witness(&p2pkh_witness, signable_bytes);
+        let p2pkh_result = p2pkh_address.verify_bytes_against_witness(
+            &p2pkh_witness,
+            signable_bytes,
+            PlatformVersion::latest(),
+        );
         assert!(
             p2pkh_result.is_ok(),
             "P2PKH redemption should succeed: {:?}",
@@ -1163,7 +1200,11 @@ mod tests {
             signatures: vec![BinaryData::new(p2sh_sig0), BinaryData::new(p2sh_sig2)],
             redeem_script: BinaryData::new(redeem_script),
         };
-        let p2sh_result = p2sh_address.verify_bytes_against_witness(&p2sh_witness, signable_bytes);
+        let p2sh_result = p2sh_address.verify_bytes_against_witness(
+            &p2sh_witness,
+            signable_bytes,
+            PlatformVersion::latest(),
+        );
         assert!(
             p2sh_result.is_ok(),
             "P2SH redemption should succeed: {:?}",
@@ -1192,7 +1233,11 @@ mod tests {
             signatures: vec![BinaryData::new(vec![0x30, 0x44])],
             redeem_script: BinaryData::new(vec![0x52]),
         };
-        let result = p2pkh_address.verify_bytes_against_witness(&p2sh_witness, signable_bytes);
+        let result = p2pkh_address.verify_bytes_against_witness(
+            &p2sh_witness,
+            signable_bytes,
+            PlatformVersion::latest(),
+        );
         assert!(result.is_err());
         assert!(result
             .unwrap_err()
@@ -1203,7 +1248,11 @@ mod tests {
         let p2pkh_witness = AddressWitness::P2pkh {
             signature: BinaryData::new(vec![0x30, 0x44]),
         };
-        let result = p2sh_address.verify_bytes_against_witness(&p2pkh_witness, signable_bytes);
+        let result = p2sh_address.verify_bytes_against_witness(
+            &p2pkh_witness,
+            signable_bytes,
+            PlatformVersion::latest(),
+        );
         assert!(result.is_err());
         assert!(result
             .unwrap_err()

packages/rs-dpp/src/state_transition/mod.rs

@@ -2404,6 +2404,7 @@ impl StateTransition {
     fn verify_ecdsa_hash_160_signature_by_public_key_hash(
         &self,
         public_key_hash: &[u8],
+        platform_version: &PlatformVersion,
     ) -> Result<(), ProtocolError> {
         let Some(signature) = self.signature() else {
             return Err(ProtocolError::InvalidVerificationWrongNumberOfElements {
@@ -2419,15 +2420,19 @@ impl StateTransition {
         }
         let data = self.signable_bytes()?;
         let data_hash = double_sha(data);
-        signer::verify_hash_signature(&data_hash, signature.as_slice(), public_key_hash).map_err(
-            |e| {
-                ProtocolError::from(ConsensusError::SignatureError(
-                    SignatureError::InvalidStateTransitionSignatureError(
-                        InvalidStateTransitionSignatureError::new(e.to_string()),
-                    ),
-                ))
-            },
-        )
+        ecdsa::verify_hash_signature(
+            &data_hash,
+            signature.as_slice(),
+            public_key_hash,
+            platform_version,
+        )?
+        .map_err(|e| {
+            ProtocolError::from(ConsensusError::SignatureError(
+                SignatureError::InvalidStateTransitionSignatureError(
+                    InvalidStateTransitionSignatureError::new(e.to_string()),
+                ),
+            ))
+        })
     }
 
     #[cfg(feature = "state-transition-validation")]
@@ -3428,6 +3433,91 @@ mod tests {
         }
     }
 
+    // An ECDSA_HASH160 key is checked by recovering the public key, which takes any S and either
+    // header form. From protocol version 15 (`verify_hash_signature` 1) the two twins a third
+    // party can derive from a valid signature, the high-S one and the one whose header names an
+    // uncompressed key, are refused; protocol version 14 keeps accepting them for replay.
+    // Masternode votes, signed by an ECDSA_HASH160 VOTING key, take the same path.
+    #[cfg(feature = "state-transition-signing")]
+    #[test]
+    fn should_refuse_an_ecdsa_hash160_signature_twin_from_protocol_version_15() {
+        use crate::identity::identity_public_key::v0::IdentityPublicKeyV0;
+        use crate::tests::utils::{high_s_twin, uncompressed_header_twin};
+        use secp256k1::{PublicKey as SecpPublicKey, SecretKey};
+
+        let bls = crate::bls::native_bls::NativeBlsModule;
+        let private_key = [0x21; 32];
+        let public_key = SecpPublicKey::from_secret_key(
+            &SecretKey::from_secret_bytes(private_key).expect("a valid ECDSA private key"),
+        );
+        let key_hash = ripemd160_sha256(&public_key.serialize());
+
+        let latest = PlatformVersion::latest();
+        let last_without_the_check = PlatformVersion::get(14).expect("protocol version 14");
+
+        for (transition, key_id, purpose, label) in [
+            (sample_transfer_st(), 11, Purpose::TRANSFER, "transfer"),
+            (
+                sample_masternode_vote_st(),
+                5,
+                Purpose::VOTING,
+                "masternode vote",
+            ),
+        ] {
+            let key: IdentityPublicKey = IdentityPublicKeyV0 {
+                id: key_id,
+                purpose,
+                security_level: SecurityLevel::CRITICAL,
+                key_type: KeyType::ECDSA_HASH160,
+                data: key_hash.to_vec().into(),
+                ..Default::default()
+            }
+            .into();
+
+            let mut signed = transition;
+            signed
+                .sign_by_private_key(&private_key, KeyType::ECDSA_HASH160, &bls)
+                .expect("the key signs");
+            let signature = signed.signature().expect("signed").to_vec();
+
+            for platform_version in [latest, last_without_the_check] {
+                signed
+                    .verify_identity_signed_signature(&key, &bls, platform_version)
+                    .unwrap_or_else(|e| {
+                        panic!(
+                            "{label}: the low-S signature verifies at protocol version {}: {e:?}",
+                            platform_version.protocol_version
+                        )
+                    });
+            }
+            for (twin, twin_label) in [
+                (high_s_twin(&signature), "high-S twin"),
+                (
+                    uncompressed_header_twin(&signature),
+                    "uncompressed header twin",
+                ),
+            ] {
+                let mut malleated = signed.clone();
+                malleated.set_signature(BinaryData::new(twin));
+                malleated
+                    .verify_identity_signed_signature(&key, &bls, last_without_the_check)
+                    .unwrap_or_else(|e| {
+                        panic!("{label}: protocol version 14 accepts the {twin_label}: {e:?}")
+                    });
+                let error = malleated
+                    .verify_identity_signed_signature(&key, &bls, latest)
+                    .expect_err("the twin is refused");
+                assert!(
+                    matches!(&error, ProtocolError::ConsensusError(consensus_error)
+                    if matches!(&**consensus_error, ConsensusError::SignatureError(
+                        SignatureError::InvalidStateTransitionSignatureError(_)
+                    ))),
+                    "{label}, {twin_label}: {error:?}"
+                );
+            }
+        }
+    }
+
     fn sample_batch_st_with_delete() -> StateTransition {
         let base = DocumentBaseTransition::V0(DocumentBaseTransitionV0 {
             id: Identifier::from([1u8; 32]),

packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funding_from_asset_lock_transition/signing_tests.rs

@@ -145,7 +145,7 @@ async fn try_from_asset_lock_with_signer_and_private_key_signs_single_p2pkh_inpu
         .expect("signable_bytes");
     let input_addr = v0.inputs.keys().next().expect("one input");
     input_addr
-        .verify_bytes_against_witness(&v0.input_witnesses[0], &signable)
+        .verify_bytes_against_witness(&v0.input_witnesses[0], &signable, PlatformVersion::latest())
         .expect("witness should verify against signable bytes");
 }
 
@@ -187,7 +187,7 @@ async fn try_from_asset_lock_with_signer_and_private_key_signs_multiple_inputs()
         .signable_bytes()
         .expect("signable_bytes");
     for (addr, witness) in v0.inputs.keys().zip(v0.input_witnesses.iter()) {
-        addr.verify_bytes_against_witness(witness, &signable)
+        addr.verify_bytes_against_witness(witness, &signable, PlatformVersion::latest())
             .expect("witness should verify against signable bytes");
     }
 }
@@ -299,6 +299,6 @@ async fn try_from_asset_lock_with_signers_produces_matching_signature() {
         .expect("signable_bytes");
     let input_addr = v0.inputs.keys().next().expect("one input");
     input_addr
-        .verify_bytes_against_witness(&v0.input_witnesses[0], &signable)
+        .verify_bytes_against_witness(&v0.input_witnesses[0], &signable, PlatformVersion::latest())
         .expect("witness should verify against signable bytes");
 }

packages/rs-dpp/src/state_transition/state_transitions/address_funds/address_funds_transfer_transition/signing_tests.rs

@@ -241,7 +241,7 @@ fn verify_transition_signatures(
         .enumerate()
     {
         let _operations = address
-            .verify_bytes_against_witness(witness, &signable_bytes)
+            .verify_bytes_against_witness(witness, &signable_bytes, PlatformVersion::latest())
             .map_err(|e| {
                 ProtocolError::Generic(format!("Witness {} verification failed: {}", i, e))
             })?;