From d9fe3402fda853030fc7ff7a6348b6951e56c90a Mon Sep 17 00:00:00 2001 From: JCSanPedro Date: Tue, 8 Oct 2024 13:21:37 +1300 Subject: [PATCH] TRN-212 Exclude non-owner proxied FP transactions (#882) * filter non-owner futurepass calls * add additional check to ensure owner can still make whitelisted proxy calls --- pallet/futurepass/src/mock.rs | 2 + pallet/futurepass/src/tests.rs | 83 ++++++++++++++++++++++++++++++++++ runtime/src/impls.rs | 4 ++ 3 files changed, 89 insertions(+) diff --git a/pallet/futurepass/src/mock.rs b/pallet/futurepass/src/mock.rs index 6ce182fb7..e713bbc9a 100644 --- a/pallet/futurepass/src/mock.rs +++ b/pallet/futurepass/src/mock.rs @@ -63,6 +63,8 @@ impl InstanceFilter for ProxyType { ) { return false; } + + return self == &ProxyType::Owner; } match self { _ => true, diff --git a/pallet/futurepass/src/tests.rs b/pallet/futurepass/src/tests.rs index 5fc731234..65d4c71da 100644 --- a/pallet/futurepass/src/tests.rs +++ b/pallet/futurepass/src/tests.rs @@ -1631,6 +1631,89 @@ fn proxy_extrinsic_to_proxy_pallet_fails() { }); } +#[test] +fn cannot_bypass_proxy_extrinsic_via_proxy() { + let funder = create_account(1); + let endowed = [(funder, 1_000_000)]; + + TestExt::::default() + .with_balances(&endowed) + .with_xrp_balances(&endowed) + .build() + .execute_with(|| { + let owner = create_account(2); + let (signer, delegate) = create_random_pair(); + + let proxy_type = ProxyType::Any; + let deadline = 200; + + // fund owner + transfer_funds( + MOCK_NATIVE_ASSET_ID, + &funder, + &owner, + FP_CREATION_RESERVE + FP_DELEGATE_RESERVE + 1, + ); + + // create FP for owner + assert_ok!(Futurepass::create(RuntimeOrigin::signed(owner), owner)); + let futurepass = Holders::::get(&owner).unwrap(); + + let signature = signer + .sign_prehashed( + &Futurepass::generate_add_delegate_eth_signed_message( + &futurepass, + &delegate, + &proxy_type, + &deadline, + ) + .unwrap() + .1, + ) + .0; + + // register delegate + assert_ok!(Futurepass::register_delegate_with_signature( + RuntimeOrigin::signed(owner), + futurepass, + delegate, + proxy_type, + deadline, + signature, + )); + + // fund futurepass + transfer_funds(MOCK_NATIVE_ASSET_ID, &funder, &futurepass, 1000); + + let other = create_account(4); + let inner_call = Box::new(MockCall::Futurepass(Call::transfer_futurepass { + current_owner: owner, + new_owner: Some(other), + })); + assert_ok!(pallet_proxy::Pallet::::proxy( + RuntimeOrigin::signed(delegate), + futurepass, + None, + inner_call.clone() + )); + + // the delegate tried to transfer the futurepass, but because it was + // was filtered, the futurepass should still be owned by the original + // owner + assert_eq!(futurepass, Holders::::get(&owner).unwrap()); + assert!(Holders::::get(&other).is_none()); + + // validate the owner is still able to make whitelisted proxy calls + assert_ok!(pallet_proxy::Pallet::::proxy( + RuntimeOrigin::signed(owner), + futurepass, + None, + inner_call + )); + assert_eq!(futurepass, Holders::::get(&other).unwrap()); + }); +} + #[test] fn proxy_extrinsic_failures_common() { let funder = create_account(1); diff --git a/runtime/src/impls.rs b/runtime/src/impls.rs index 4aeca2cd6..c55de9db5 100644 --- a/runtime/src/impls.rs +++ b/runtime/src/impls.rs @@ -786,6 +786,10 @@ impl InstanceFilter for ProxyType { ) { return false; } + + // the whitelisted calls above should only be able to be called by + // the owner of the futurepass + return self == &ProxyType::Owner; } match self { ProxyType::Owner => true,