CVE-2023-45130 (Rust, published 2023-10-13T13:15Z)
Description: value: Frontier is Substrate's Ethereum compatibility layer. Prior to commit aea528198b3b226e0d20cce878551fd4c0e3d5d0, at the end of a contract execution, when opcode SUICIDE marks a contract to be deleted, the software uses `storage::remove_prefix` (now renamed to `storage::clear_prefix`) to remove all storages associated with it. This is a single IO primitive call passing the WebAssembly boundary. For large contracts, the call (without providing a `limit` parameter) can be slow. In addition, for parachains, all storages to be deleted will be part of the PoV, which easily exceed relay chain PoV size limit. On the other hand, Frontier's maintainers only charge a fixed cost for opcode SUICIDE. The maintainers consider the severity of this issue high, because an attacker can craft a contract with a lot of storage values on a parachain, and then call opcode SUICIDE on the contract. If the transaction makes into a parachain block, the parachain will then stall because the PoV size will exceed relay chain's limit. This is especially an issue for XCM transactions, because they can't be skipped. Commit aea528198b3b226e0d20cce878551fd4c0e3d5d0 contains a patch for this issue. For parachains, it's recommended to issue an emergency runtime upgrade as soon as possible. For standalone chains, the impact is less severe because the issue mainly affects PoV sizes. It's recommended to issue a normal runtime upgrade as soon as possible. There are no known workarounds.
Severity: nan (CVSS v3 base score: 7.5)
Weakness: CWE-770 \u2014 Allocation of Resources Without Limits or Throttling
The product allocates a reusable resource or group of resources on behalf of an actor without imposing any restrictions on the size or number of resources that can be allocated, in violation of the intended security policy for that actor.
Repository: https://github.com/paritytech/frontier
Files changed:
frame/evm/src/lib.rs
frame/evm/src/tests.rs
Commit message: Initial support for asynchronous suiciding (#1212)

* Initial support for asynchronous suiciding

* Use contains_key instead of is_some

Co-authored-by: Xiliang Chen <xlchen1291@gmail.com>

* Fix compile

* Remove unnecessary Suicided check

* Don't call dec_sufficients

* Handle pre-EIP161 case

* Add extra safeguard

* Always inc nonce

* Fix tests

---------

Co-authored-by: Xiliang Chen <xlchen1291@gmail.com>

Vulnerable code:
			let _ = frame_system::Pallet::<T>::dec_sufficients(&account_id);
		#[allow(deprecated)]
		let _ = <AccountStorages<T>>::remove_prefix(address, None);
		// We decreased the sufficient reference by 1 on removing the account.
		assert_eq!(account_2.sufficients, 0);

Fixed code:

	#[pallet::storage]
	pub type Suicided<T: Config> = StorageMap<_, Blake2_128Concat, H160, (), OptionQuery>;
			// Remember to call `dec_sufficients` when clearing Suicided.
			<Suicided<T>>::insert(address, ());

			// In theory, we can always have pre-EIP161 contracts, so we
			// make sure the account nonce is at least one.
			frame_system::Pallet::<T>::inc_account_nonce(&account_id);
		if <Suicided<T>>::contains_key(address) {
			// This branch should never trigger, because when Suicided
			// contains an address, then its nonce will be at least one,
			// which causes CreateCollision error in EVM, but we add it
			// here for safeguard.
			return;
		}

		assert_eq!(account_2.sufficients, 1);