pallet-identity: Be more paranoid ;)

frame/identity/src/lib.rs

@@ -79,7 +79,7 @@ mod types;
 pub mod weights;
 
 use frame_support::traits::{BalanceStatus, Currency, OnUnbalanced, ReservableCurrency};
-use sp_runtime::traits::{AppendZerosInput, Saturating, StaticLookup, Zero};
+use sp_runtime::traits::{AppendZerosInput, Saturating, StaticLookup, Zero, Hash};
 use sp_std::prelude::*;
 pub use weights::WeightInfo;
 
@@ -236,6 +236,8 @@ pub mod pallet {
 		NotSub,
 		/// Sub-account isn't owned by sender.
 		NotOwned,
+		/// The provided judgement was for a different identity.
+		JudgementForDifferentIdentity,
 	}
 
 	#[pallet::event]
@@ -746,6 +748,7 @@ pub mod pallet {
 		/// - `target`: the account whose identity the judgement is upon. This must be an account
 		///   with a registered identity.
 		/// - `judgement`: the judgement of the registrar of index `reg_index` about `target`.
+		/// - `identity`: The hash of the [`IdentityInfo`] for that the judgement is provided.
 		///
 		/// Emits `JudgementGiven` if successful.
 		///
@@ -765,17 +768,22 @@ pub mod pallet {
 			#[pallet::compact] reg_index: RegistrarIndex,
 			target: AccountIdLookupOf<T>,
 			judgement: Judgement<BalanceOf<T>>,
+			identity: T::Hash,
 		) -> DispatchResultWithPostInfo {
 			let sender = ensure_signed(origin)?;
 			let target = T::Lookup::lookup(target)?;
 			ensure!(!judgement.has_deposit(), Error::<T>::InvalidJudgement);
 			<Registrars<T>>::get()
 				.get(reg_index as usize)
 				.and_then(Option::as_ref)
-				.and_then(|r| if r.account == sender { Some(r) } else { None })
+				.filter(|r| r.account == sender)
 				.ok_or(Error::<T>::InvalidIndex)?;
 			let mut id = <IdentityOf<T>>::get(&target).ok_or(Error::<T>::InvalidTarget)?;
 
+			if T::Hashing::hash_of(&id.info) != identity {
+				return Err(Error::<T>::JudgementForDifferentIdentity.into())
+			}
+
 			let item = (reg_index, judgement);
 			match id.judgements.binary_search_by_key(&reg_index, |x| x.0) {
 				Ok(position) => {

frame/identity/src/tests.rs

@@ -279,27 +279,70 @@ fn registration_should_work() {
 fn uninvited_judgement_should_work() {
 	new_test_ext().execute_with(|| {
 		assert_noop!(
-			Identity::provide_judgement(Origin::signed(3), 0, 10, Judgement::Reasonable),
+			Identity::provide_judgement(
+				Origin::signed(3),
+				0,
+				10,
+				Judgement::Reasonable,
+				H256::random()
+			),
 			Error::<Test>::InvalidIndex
 		);
 
 		assert_ok!(Identity::add_registrar(Origin::signed(1), 3));
 		assert_noop!(
-			Identity::provide_judgement(Origin::signed(3), 0, 10, Judgement::Reasonable),
+			Identity::provide_judgement(
+				Origin::signed(3),
+				0,
+				10,
+				Judgement::Reasonable,
+				H256::random()
+			),
 			Error::<Test>::InvalidTarget
 		);
 
 		assert_ok!(Identity::set_identity(Origin::signed(10), Box::new(ten())));
 		assert_noop!(
-			Identity::provide_judgement(Origin::signed(10), 0, 10, Judgement::Reasonable),
+			Identity::provide_judgement(
+				Origin::signed(3),
+				0,
+				10,
+				Judgement::Reasonable,
+				H256::random()
+			),
+			Error::<Test>::JudgementForDifferentIdentity
+		);
+
+		let identity_hash = BlakeTwo256::hash_of(&ten());
+
+		assert_noop!(
+			Identity::provide_judgement(
+				Origin::signed(10),
+				0,
+				10,
+				Judgement::Reasonable,
+				identity_hash
+			),
 			Error::<Test>::InvalidIndex
 		);
 		assert_noop!(
-			Identity::provide_judgement(Origin::signed(3), 0, 10, Judgement::FeePaid(1)),
+			Identity::provide_judgement(
+				Origin::signed(3),
+				0,
+				10,
+				Judgement::FeePaid(1),
+				identity_hash
+			),
 			Error::<Test>::InvalidJudgement
 		);
 
-		assert_ok!(Identity::provide_judgement(Origin::signed(3), 0, 10, Judgement::Reasonable));
+		assert_ok!(Identity::provide_judgement(
+			Origin::signed(3),
+			0,
+			10,
+			Judgement::Reasonable,
+			identity_hash
+		));
 		assert_eq!(Identity::identity(10).unwrap().judgements, vec![(0, Judgement::Reasonable)]);
 	});
 }
@@ -309,7 +352,13 @@ fn clearing_judgement_should_work() {
 	new_test_ext().execute_with(|| {
 		assert_ok!(Identity::add_registrar(Origin::signed(1), 3));
 		assert_ok!(Identity::set_identity(Origin::signed(10), Box::new(ten())));
-		assert_ok!(Identity::provide_judgement(Origin::signed(3), 0, 10, Judgement::Reasonable));
+		assert_ok!(Identity::provide_judgement(
+			Origin::signed(3),
+			0,
+			10,
+			Judgement::Reasonable,
+			BlakeTwo256::hash_of(&ten())
+		));
 		assert_ok!(Identity::clear_identity(Origin::signed(10)));
 		assert_eq!(Identity::identity(10), None);
 	});
@@ -411,7 +460,13 @@ fn cancelling_requested_judgement_should_work() {
 		assert_eq!(Balances::free_balance(10), 90);
 		assert_noop!(Identity::cancel_request(Origin::signed(10), 0), Error::<Test>::NotFound);
 
-		assert_ok!(Identity::provide_judgement(Origin::signed(3), 0, 10, Judgement::Reasonable));
+		assert_ok!(Identity::provide_judgement(
+			Origin::signed(3),
+			0,
+			10,
+			Judgement::Reasonable,
+			BlakeTwo256::hash_of(&ten())
+		));
 		assert_noop!(
 			Identity::cancel_request(Origin::signed(10), 0),
 			Error::<Test>::JudgementGiven
@@ -438,7 +493,13 @@ fn requesting_judgement_should_work() {
 			Identity::request_judgement(Origin::signed(10), 0, 10),
 			Error::<Test>::StickyJudgement
 		);
-		assert_ok!(Identity::provide_judgement(Origin::signed(3), 0, 10, Judgement::Erroneous));
+		assert_ok!(Identity::provide_judgement(
+			Origin::signed(3),
+			0,
+			10,
+			Judgement::Erroneous,
+			BlakeTwo256::hash_of(&ten())
+		));
 		// Registrar got their payment now.
 		assert_eq!(Balances::free_balance(3), 20);
 
@@ -453,7 +514,13 @@ fn requesting_judgement_should_work() {
 		assert_ok!(Identity::request_judgement(Origin::signed(10), 1, 10));
 
 		// Re-requesting after the judgement has been reduced works.
-		assert_ok!(Identity::provide_judgement(Origin::signed(3), 0, 10, Judgement::OutOfDate));
+		assert_ok!(Identity::provide_judgement(
+			Origin::signed(3),
+			0,
+			10,
+			Judgement::OutOfDate,
+			BlakeTwo256::hash_of(&ten())
+		));
 		assert_ok!(Identity::request_judgement(Origin::signed(10), 0, 10));
 	});
 }