CVE-2021-39193: Improper Validation of Specified Quantity in Input (CWE-1284)
value: Frontier is Substrate's Ethereum compatibility layer. Prior to commit number 0b962f218f0cdd796dadfe26c3f09e68f7861b26, a bug in `pallet-ethereum` can cause invalid transactions to be included in the Ethereum block state in `pallet-ethereum` due to not validating the input data size. Any invalid transactions included this way have no possibility to alter the internal Ethereum or Substrate state. The transaction will appear to have be included, but is of no effect as it is rejected by the EVM engine. The impact is further limited by Substrate extrinsic size constraints. A patch is available in commit number 0b962f218f0cdd796dadfe26c3f09e68f7861b26. There are no workarounds aside from applying the patch.
CWE description: The product receives input that is expected to specify a quantity (such as size or length), but it does not validate or incorrectly validates that the quantity has the required properties.
CVSS v3 base score: 5.3
Published: 2021-09-03T18:15Z
Repository: https://github.com/paritytech/frontier
Language: Rust
Files changed: frame/ethereum/src/lib.rs, ts-tests/tests/test-transaction-cost.ts
Commit date: 2021-09-03 17:48:21 +0200
Commit message: Add transaction cost pre-validation (#465)


Diff: diff --git a/frame/ethereum/src/lib.rs b/frame/ethereum/src/lib.rs
index c5f702a..23cad2d 100644
--- a/frame/ethereum/src/lib.rs
+++ b/frame/ethereum/src/lib.rs
@@ -188,10 +188,31 @@ pub mod pallet {
 	impl<T: Config> ValidateUnsigned for Pallet<T> {
 		type Call = Call<T>;
 
 		fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity {
 			if let Call::transact(transaction) = call {
+				// We must ensure a transaction can pay the cost of its data bytes.
+				// If it can't it should not be included in a block.
+				let mut gasometer = evm::gasometer::Gasometer::new(
+					transaction.gas_limit.low_u64(),
+					<T as pallet_evm::Config>::config(),
+				);
+				let transaction_cost = match transaction.action {
+					TransactionAction::Call(_) => {
+						evm::gasometer::call_transaction_cost(&transaction.input)
+					}
+					TransactionAction::Create => {
+						evm::gasometer::create_transaction_cost(&transaction.input)
+					}
+				};
+				if gasometer.record_transaction(transaction_cost).is_err() {
+					return InvalidTransaction::Custom(
+						TransactionValidationError::InvalidGasLimit as u8,
+					)
+					.into();
+				}
+
 				if let Some(chain_id) = transaction.signature.chain_id() {
 					if chain_id != T::ChainId::get() {
 						return InvalidTransaction::Custom(
 							TransactionValidationError::InvalidChainId as u8,
 						)
diff --git a/ts-tests/tests/test-transaction-cost.ts b/ts-tests/tests/test-transaction-cost.ts
new file mode 100644
index 0000000..f1a5307
--- /dev/null
+++ b/ts-tests/tests/test-transaction-cost.ts
@@ -0,0 +1,21 @@
+import { expect } from "chai";
+import { step } from "mocha-steps";
+
+import { describeWithFrontier, customRequest } from "./util";
+
+describeWithFrontier("Frontier RPC (Transaction cost)", (context) => {
+
+	step("should take transaction cost into account and not submit it to the pool", async function () {
+		// Simple transfer with gas limit 0 manually signed to prevent web3 from rejecting client-side.
+		const tx = await customRequest(context.web3, "eth_sendRawTransaction", [
+			"0xf86180843b9aca00809412cb274aad8251c875c0bf6872b67d9983e53fdd01801ca00e28ba2dd3c5a3fd467\
+			d4afd7aefb4a34b373314fff470bb9db743a84d674a0aa06e5994f2d07eafe1c37b4ce5471caecec29011f6f5b\
+			f0b1a552c55ea348df35f",
+		]);
+		let msg =
+			"submit transaction to pool failed: Pool(InvalidTransaction(InvalidTransaction::Custom(3)))";
+		expect(tx.error).to.include({
+			message: msg,
+		});
+	});
+});

Fixed code: 				// We must ensure a transaction can pay the cost of its data bytes.
				// If it can't it should not be included in a block.
				let mut gasometer = evm::gasometer::Gasometer::new(
					transaction.gas_limit.low_u64(),
					<T as pallet_evm::Config>::config(),
				);
				let transaction_cost = match transaction.action {
					TransactionAction::Call(_) => {
						evm::gasometer::call_transaction_cost(&transaction.input)
					}
					TransactionAction::Create => {
						evm::gasometer::create_transaction_cost(&transaction.input)
					}
				};
				if gasometer.record_transaction(transaction_cost).is_err() {
					return InvalidTransaction::Custom(
						TransactionValidationError::InvalidGasLimit as u8,
					)
					.into();
				}

import { expect } from "chai";
import { step } from "mocha-steps";

import { describeWithFrontier, customRequest } from "./util";

describeWithFrontier("Frontier RPC (Transaction cost)", (context) => {

	step("should take transaction cost into account and not submit it to the pool", async function () {
		// Simple transfer with gas limit 0 manually signed to prevent web3 from rejecting client-side.
		const tx = await customRequest(context.web3, "eth_sendRawTransaction", [
			"0xf86180843b9aca00809412cb274aad8251c875c0bf6872b67d9983e53fdd01801ca00e28ba2dd3c5a3fd467\
			d4afd7aefb4a34b373314fff470bb9db743a84d674a0aa06e5994f2d07eafe1c37b4ce5471caecec29011f6f5b\
			f0b1a552c55ea348df35f",
		]);
		let msg =
			"submit transaction to pool failed: Pool(InvalidTransaction(InvalidTransaction::Custom(3)))";
		expect(tx.error).to.include({
			message: msg,
		});
	});
});