diff --git a/polkadot/xcm/pallet-xcm/precompiles/src/lib.rs b/polkadot/xcm/pallet-xcm/precompiles/src/lib.rs index 47b05f6397f94..64ed91f46f4f1 100644 --- a/polkadot/xcm/pallet-xcm/precompiles/src/lib.rs +++ b/polkadot/xcm/pallet-xcm/precompiles/src/lib.rs @@ -82,6 +82,8 @@ where }; match input { + IXcmCalls::send(_) | IXcmCalls::execute(_) if env.is_read_only() => + Err(Error::Error(pallet_revive::Error::::StateChangeDenied.into())), IXcmCalls::send(IXcm::sendCall { destination, message }) => { let _ = env.charge(::WeightInfo::send())?; diff --git a/prdoc/pr_10080.prdoc b/prdoc/pr_10080.prdoc new file mode 100644 index 0000000000000..a0490308d8a5b --- /dev/null +++ b/prdoc/pr_10080.prdoc @@ -0,0 +1,16 @@ +title: 'precompiles: Enforce state mutability' +doc: +- audience: Runtime Dev + description: |- + `pallet-assets-precompile`, `pallet-xcm-precompiles` and revive builtin precompile implementations currently violate [Solidity state mutability](https://docs.soliditylang.org/en/latest/grammar.html#syntax-rule-SolidityParser.stateMutability), potentially introducing a new attack vector. This PR implements corresponding checks at the function dispatch. + + Could be enforced in `pallet-revive`, however: + 1. Adding something like a `const MUTATES: bool` to the `Precompile` trait won't help because whether the call is mutating or not depends on the [Solidity function selector.](https://docs.soliditylang.org/en/latest/abi-spec.html#function-selector). + 2. Alloy, which we are using to parse the interface definitions prior to calling precompile implementations, doesn't provide a mapping from function selector to its mutability [modifier](https://docs.soliditylang.org/en/latest/cheatsheet.html#modifiers). +crates: +- name: pallet-assets-precompiles + bump: patch +- name: pallet-xcm-precompiles + bump: patch +- name: pallet-revive + bump: patch diff --git a/substrate/frame/assets/precompiles/src/lib.rs b/substrate/frame/assets/precompiles/src/lib.rs index c9f41a4581b86..17cdd49fea30d 100644 --- a/substrate/frame/assets/precompiles/src/lib.rs +++ b/substrate/frame/assets/precompiles/src/lib.rs @@ -108,6 +108,10 @@ where let asset_id = PrecompileConfig::AssetIdExtractor::asset_id_from_address(address)?.into(); match input { + IERC20Calls::transfer(_) | IERC20Calls::approve(_) | IERC20Calls::transferFrom(_) + if env.is_read_only() => + Err(Error::Error(pallet_revive::Error::::StateChangeDenied.into())), + IERC20Calls::transfer(call) => Self::transfer(asset_id, call, env), IERC20Calls::totalSupply(_) => Self::total_supply(asset_id, env), IERC20Calls::balanceOf(call) => Self::balance_of(asset_id, call, env), diff --git a/substrate/frame/revive/src/precompiles/builtin/storage.rs b/substrate/frame/revive/src/precompiles/builtin/storage.rs index de387df5d28ba..682624ad93094 100644 --- a/substrate/frame/revive/src/precompiles/builtin/storage.rs +++ b/substrate/frame/revive/src/precompiles/builtin/storage.rs @@ -54,6 +54,10 @@ impl BuiltinPrecompile for Storage { use IStorage::IStorageCalls; let max_size = limits::STORAGE_BYTES; match input { + IStorageCalls::clearStorage(_) | IStorageCalls::takeStorage(_) + if env.is_read_only() => + Err(Error::Error(crate::Error::::StateChangeDenied.into())), + IStorageCalls::clearStorage(IStorage::clearStorageCall { flags, key, isFixedKey }) => { let transient = is_transient(*flags) .map_err(|_| Error::Revert("invalid storage flag".into()))?;