Skip to content

Commit 9c21405

Browse files
[pallet-assets] Reject delegatecall into pallet-assets ERC20 precompile (#11676)
There is no legitimate use case for delegatecalling into the asset precompile. This matches the precedent set by the Storage precompile, which already enforces a delegatecall check (in the opposite direction — it *requires* delegatecall). ## Changes - `lib.rs`: Add `ERR_DELEGATECALL_DENIED` const and `is_delegate_call()` guard before any dispatch logic - `tests.rs`: Add `delegatecall_is_rejected` test using the `Caller.sol` fixture ## Test plan - [x] `cargo test -p pallet-assets-precompiles` — all 67 tests pass - [x] `delegatecall_is_rejected` verifies the guard rejects delegatecall via the `Caller` fixture contract --------- Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent 0ec94c9 commit 9c21405

File tree

3 files changed

+80
-0
lines changed

3 files changed

+80
-0
lines changed

prdoc/pr_11676.prdoc

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
title: '[pallet-assets] Reject delegatecall into pallet-assets ERC20 precompile'
2+
doc:
3+
- audience: Runtime Dev
4+
description: "There is no legitimate use case for delegatecalling into the asset\
5+
\ precompile. This matches the precedent set by the Storage precompile, which\
6+
\ already enforces a delegatecall check (in the opposite direction \u2014 it *requires*\
7+
\ delegatecall).\n\n## Changes\n\n- `lib.rs`: Add `ERR_DELEGATECALL_DENIED` const\
8+
\ and `is_delegate_call()` guard before any dispatch logic\n- `tests.rs`: Add\
9+
\ `delegatecall_is_rejected` test using the `Caller.sol` fixture\n\n## Test plan\n\
10+
\n- [x] `cargo test -p pallet-assets-precompiles` \u2014 all 67 tests pass\n-\
11+
\ [x] `delegatecall_is_rejected` verifies the guard rejects delegatecall via the\
12+
\ `Caller` fixture contract"
13+
crates:
14+
- name: pallet-assets-precompiles
15+
bump: minor

substrate/frame/assets/precompiles/src/lib.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,10 @@ where
160160
input: &Self::Interface,
161161
env: &mut impl Ext<T = Self::T>,
162162
) -> Result<Vec<u8>, Error> {
163+
if env.is_delegate_call() {
164+
return Err(Error::Revert(Revert { reason: ERR_DELEGATECALL_DENIED.into() }));
165+
}
166+
163167
let asset_id = PrecompileConfig::AssetIdExtractor::asset_id_from_address(address)?.into();
164168
let contract_addr = H160::from(*address);
165169

@@ -197,6 +201,7 @@ where
197201
}
198202
}
199203

204+
const ERR_DELEGATECALL_DENIED: &str = "Cannot be called via delegatecall";
200205
const ERR_INVALID_CALLER: &str = "Invalid caller";
201206
const ERR_BALANCE_CONVERSION_FAILED: &str = "Balance conversion failed";
202207

substrate/frame/assets/precompiles/src/tests.rs

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,7 @@ fn approve_zero_on_nonexistent_is_noop(asset_index: u16) {
532532
alloy::sol! {
533533
interface ICaller {
534534
function staticCall(address callee, bytes data, uint64 gas) external view returns (bool success, bytes output);
535+
function delegate(address callee, bytes data, uint64 gas) external returns (bool success, bytes output);
535536
}
536537
}
537538

@@ -626,3 +627,62 @@ fn domain_separator_is_staticcall_compatible(asset_index: u16) {
626627
);
627628
});
628629
}
630+
631+
#[test]
632+
fn delegatecall_is_rejected() {
633+
new_test_ext().execute_with(|| {
634+
let asset_id = 0u32;
635+
let asset_addr = H160::from(set_prefix_in_address(PRECOMPILE_ADDRESS_PREFIX));
636+
let deployer = 123456789u64;
637+
Balances::make_free_balance_be(&deployer, 1_000_000_000_000_000u64);
638+
639+
assert_ok!(Assets::force_create(RuntimeOrigin::root(), asset_id, deployer, true, 1));
640+
assert_ok!(Assets::mint(RuntimeOrigin::signed(deployer), asset_id, deployer, 1000));
641+
642+
let (init_code, _) = pallet_revive_fixtures::compile_module_with_type(
643+
"Caller",
644+
pallet_revive_fixtures::FixtureType::Solc,
645+
)
646+
.expect("Caller fixture must be compiled");
647+
let caller_addr = pallet_revive::Pallet::<Test>::bare_instantiate(
648+
RuntimeOrigin::signed(deployer),
649+
0u32.into(),
650+
TransactionLimits::WeightAndDeposit {
651+
weight_limit: Weight::MAX,
652+
deposit_limit: u64::MAX,
653+
},
654+
Code::Upload(init_code),
655+
vec![],
656+
None,
657+
&ExecConfig::new_substrate_tx(),
658+
)
659+
.result
660+
.expect("Caller deployment must succeed")
661+
.addr;
662+
663+
let calldata = ICaller::delegateCall {
664+
callee: alloy::primitives::Address::from(asset_addr.0),
665+
data: IERC20::totalSupplyCall {}.abi_encode().into(),
666+
gas: u64::MAX,
667+
}
668+
.abi_encode();
669+
670+
let result = pallet_revive::Pallet::<Test>::bare_call(
671+
RuntimeOrigin::signed(deployer),
672+
caller_addr,
673+
0u32.into(),
674+
TransactionLimits::WeightAndDeposit {
675+
weight_limit: Weight::MAX,
676+
deposit_limit: u64::MAX,
677+
},
678+
calldata,
679+
&ExecConfig::new_substrate_tx(),
680+
)
681+
.result
682+
.expect("outer call must succeed");
683+
684+
let ret = ICaller::delegateCall::abi_decode_returns(&result.data)
685+
.expect("return must decode as (bool, bytes)");
686+
assert!(!ret.success, "DELEGATECALL to asset precompile must be rejected");
687+
});
688+
}

0 commit comments

Comments
 (0)