fix: Fix pallet-cosmwasm ecdsa signature verification bug

NoirHQ · Oct 29, 2024 · dd58109 · dd58109
1 parent 9620467
commit dd58109
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 22 deletions.
diff --git a/frame/babel/src/tests.rs b/frame/babel/src/tests.rs
@@ -19,7 +19,7 @@
 use crate::{mock::*, *};
 use frame_support::{assert_ok, traits::fungible::Inspect};
 use np_babel::EthereumAddress;
-use sp_core::ecdsa;
+use sp_core::{ecdsa, sha2_256};
 use sp_runtime::traits::AccountIdConversion;
 
 fn dev_public() -> ecdsa::Public {
@@ -79,3 +79,21 @@ fn transfer_to_nostr_address_works() {
 		assert_eq!(Balances::balance(&account), 100);
 	});
 }
+
+#[test]
+fn ecdsa_verify_prehashed() {
+	let signature = const_hex::decode("f7e0d198c62821cc5817c8e935f523308301e29819f5d882f3249b9e173a614f38000ddbff446c0abfa7c7d019dbb17072b28933fc8187c973fbf03d0459f76e").unwrap();
+	let message = const_hex::decode("0a93010a90010a1c2f636f736d6f732e62616e6b2e763162657461312e4d736753656e6412700a2d636f736d6f7331716436396e75776a393567746134616b6a677978746a39756a6d7a34773865646d7179737177122d636f736d6f7331676d6a32657861673033747467616670726b6463337438383067726d61396e776566636432771a100a057561746f6d12073130303030303012710a4e0a460a1f2f636f736d6f732e63727970746f2e736563703235366b312e5075624b657912230a21020a1091341fe5664bfa1782d5e04779689068c916b04cb365ec3153755684d9a112040a020801121f0a150a057561746f6d120c3838363838303030303030301080c0f1c59495141a1174686574612d746573746e65742d30303120ad8a2e").unwrap();
+	let message_hash = sha2_256(&message);
+	let public_key =
+		const_hex::decode("020a1091341fe5664bfa1782d5e04779689068c916b04cb365ec3153755684d9a1")
+			.unwrap();
+
+	new_test_ext().execute_with(|| {
+		assert!(pallet_cosmwasm::Pallet::<Test>::do_secp256k1_verify(
+			&message_hash,
+			&signature,
+			&public_key
+		));
+	});
+}
diff --git a/frame/cosmos/x/auth/src/sigverify.rs b/frame/cosmos/x/auth/src/sigverify.rs
@@ -219,14 +219,13 @@ pub mod tests {
 
 	#[test]
 	fn ecdsa_verify_test() {
-		let sig = const_hex::decode(b"f7e0d198c62821cc5817c8e935f523308301e29819f5d882f3249b9e173a614f38000ddbff446c0abfa7c7d019dbb17072b28933fc8187c973fbf03d0459f76e").unwrap();
-		let message = const_hex::decode(b"0a93010a90010a1c2f636f736d6f732e62616e6b2e763162657461312e4d736753656e6412700a2d636f736d6f7331716436396e75776a393567746134616b6a677978746a39756a6d7a34773865646d7179737177122d636f736d6f7331676d6a32657861673033747467616670726b6463337438383067726d61396e776566636432771a100a057561746f6d12073130303030303012710a4e0a460a1f2f636f736d6f732e63727970746f2e736563703235366b312e5075624b657912230a21020a1091341fe5664bfa1782d5e04779689068c916b04cb365ec3153755684d9a112040a020801121f0a150a057561746f6d120c3838363838303030303030301080c0f1c59495141a1174686574612d746573746e65742d30303120ad8a2e").unwrap();
+		let signature = const_hex::decode("f7e0d198c62821cc5817c8e935f523308301e29819f5d882f3249b9e173a614f38000ddbff446c0abfa7c7d019dbb17072b28933fc8187c973fbf03d0459f76e").unwrap();
+		let message = const_hex::decode("0a93010a90010a1c2f636f736d6f732e62616e6b2e763162657461312e4d736753656e6412700a2d636f736d6f7331716436396e75776a393567746134616b6a677978746a39756a6d7a34773865646d7179737177122d636f736d6f7331676d6a32657861673033747467616670726b6463337438383067726d61396e776566636432771a100a057561746f6d12073130303030303012710a4e0a460a1f2f636f736d6f732e63727970746f2e736563703235366b312e5075624b657912230a21020a1091341fe5664bfa1782d5e04779689068c916b04cb365ec3153755684d9a112040a020801121f0a150a057561746f6d120c3838363838303030303030301080c0f1c59495141a1174686574612d746573746e65742d30303120ad8a2e").unwrap();
 		let message_hash = sha2_256(&message);
-		let public_key = const_hex::decode(
-			b"020a1091341fe5664bfa1782d5e04779689068c916b04cb365ec3153755684d9a1",
-		)
-		.unwrap();
+		let public_key =
+			const_hex::decode("020a1091341fe5664bfa1782d5e04779689068c916b04cb365ec3153755684d9a1")
+				.unwrap();
 
-		assert!(ecdsa_verify_prehashed(&sig, &message_hash, &public_key));
+		assert!(ecdsa_verify_prehashed(&signature, &message_hash, &public_key));
 	}
 }
diff --git a/vendor/composable/cosmwasm/src/crypto.rs b/vendor/composable/cosmwasm/src/crypto.rs
@@ -37,30 +37,24 @@ impl<T: Config> Pallet<T> {
 			.map_err(|_| ())
 	}
 
-	pub(crate) fn do_secp256k1_verify(
-		message_hash: &[u8],
-		signature: &[u8],
-		public_key: &[u8],
-	) -> bool {
+	pub fn do_secp256k1_verify(message_hash: &[u8], signature: &[u8], public_key: &[u8]) -> bool {
 		let message_hash = match message_hash.try_into() {
 			Ok(message_hash) => message_hash,
 			Err(_) => return false,
 		};
 
-		// We are expecting 64 bytes long public keys but the substrate function use an
-		// additional byte for recovery id. So we insert a dummy byte.
-		let signature = {
-			let mut signature_inner = [0_u8; SUBSTRATE_ECDSA_SIGNATURE_LEN];
-			signature_inner[..SUBSTRATE_ECDSA_SIGNATURE_LEN - 1].copy_from_slice(signature);
-			ecdsa::Signature::from(signature_inner)
-		};
-
 		let public_key = match libsecp256k1::PublicKey::parse_slice(public_key, None) {
 			Ok(public_key) => ecdsa::Public::from_raw(public_key.serialize_compressed()),
 			Err(_) => return false,
 		};
 
-		sp_io::crypto::ecdsa_verify_prehashed(&signature, &message_hash, &public_key)
+		(0..=3).any(|rec_id| {
+			let mut rec_sig = [0_u8; SUBSTRATE_ECDSA_SIGNATURE_LEN];
+			rec_sig[..SUBSTRATE_ECDSA_SIGNATURE_LEN - 1].copy_from_slice(signature);
+			rec_sig[SUBSTRATE_ECDSA_SIGNATURE_LEN - 1] = rec_id;
+			let sig = ecdsa::Signature::from(rec_sig);
+			sp_io::crypto::ecdsa_verify_prehashed(&sig, message_hash, &public_key)
+		})
 	}
 
 	pub(crate) fn do_ed25519_batch_verify(