CVE-2023-28431: Incorrect Calculation (CWE-682)
value: Frontier is an Ethereum compatibility layer for Substrate. Frontier's `modexp` precompile uses `num-bigint` crate under the hood. In the implementation prior to pull request 1017, the cases for modulus being even and modulus being odd are treated separately. Odd modulus uses the fast Montgomery multiplication, and even modulus uses the slow plain power algorithm. This gas cost discrepancy was not accounted for in the `modexp` precompile, leading to possible denial of service attacks.

No fixes for `num-bigint` are currently available, and thus this issue is fixed in the short term by raising the gas costs for even modulus, and in the long term fixing it in `num-bigint` or switching to another modexp implementation. The short-term fix for Frontier is deployed at pull request 1017. There are no known workarounds aside from applying the fix.
CWE description: The product performs a calculation that generates incorrect or unintended results that are later used in security-critical decisions or resource management.
CVSS v3 base score: 7.5
Published: 2023-03-22T21:15Z
Repository: https://github.com/paritytech/frontier
Language: Rust
Files changed: frame/evm/precompile/modexp/src/lib.rs, shell.nix
Commit date: 2023-03-15 14:05:22 +0100
Commit message: Increase modexp gas cost when mod is even (#1017)

* Increase modexp gas cost when mod is even

* Fix tests
Diff: diff --git a/frame/evm/precompile/modexp/src/lib.rs b/frame/evm/precompile/modexp/src/lib.rs
index 6932df40..1b4aa66b 100644
--- a/frame/evm/precompile/modexp/src/lib.rs
+++ b/frame/evm/precompile/modexp/src/lib.rs
@@ -21,11 +21,11 @@
 extern crate alloc;
 
 use alloc::{vec, vec::Vec};
 use core::cmp::max;
 
-use num::{BigUint, FromPrimitive, One, ToPrimitive, Zero};
+use num::{BigUint, FromPrimitive, Integer, One, ToPrimitive, Zero};
 
 use fp_evm::{
 	ExitError, ExitSucceed, Precompile, PrecompileFailure, PrecompileHandle, PrecompileOutput,
 	PrecompileResult,
 };
@@ -39,10 +39,11 @@ const MIN_GAS_COST: u64 = 200;
 fn calculate_gas_cost(
 	base_length: u64,
 	mod_length: u64,
 	exponent: &BigUint,
 	exponent_bytes: &[u8],
+	mod_is_even: bool,
 ) -> u64 {
 	fn calculate_multiplication_complexity(base_length: u64, mod_length: u64) -> u64 {
 		let max_length = max(base_length, mod_length);
 		let mut words = max_length / 8;
 		if max_length % 8 > 0 {
@@ -89,10 +90,11 @@ fn calculate_gas_cost(
 	let iteration_count = calculate_iteration_count(exponent, exponent_bytes);
 	max(
 		MIN_GAS_COST,
 		multiplication_complexity * iteration_count / 3,
 	)
+	.saturating_mul(if mod_is_even { 20 } else { 1 })
 }
 
 /// Copy bytes from input to target.
 fn read_input(source: &[u8], target: &mut [u8], source_offset: &mut usize) {
 	// We move the offset by the len of the target, regardless of what we
@@ -194,11 +196,17 @@ impl Precompile for Modexp {
 			let mut mod_buf = vec![0u8; mod_len];
 			read_input(input, &mut mod_buf, &mut input_offset);
 			let modulus = BigUint::from_bytes_be(&mod_buf);
 
 			// do our gas accounting
-			let gas_cost = calculate_gas_cost(base_len as u64, mod_len as u64, &exponent, &exp_buf);
+			let gas_cost = calculate_gas_cost(
+				base_len as u64,
+				mod_len as u64,
+				&exponent,
+				&exp_buf,
+				modulus.is_even(),
+			);
 
 			handle.record_cost(gas_cost)?;
 
 			if modulus.is_zero() || modulus.is_one() {
 				BigUint::zero()
@@ -508,8 +516,8 @@ mod tests {
 
 		let mut handle = MockHandle::new(input, Some(100_000), context);
 
 		let _ = Modexp::execute(&mut handle).expect("Modexp::execute() returned error");
 
-		assert_eq!(handle.gas_used, 7104); // gas used when ran in geth
+		assert_eq!(handle.gas_used, 7104 * 20); // gas used when ran in geth (x20)
 	}
 }
diff --git a/shell.nix b/shell.nix
index 51b64ce6..b4c47b9b 100644
--- a/shell.nix
+++ b/shell.nix
@@ -1,10 +1,10 @@
 let
   mozillaOverlay =
     import (builtins.fetchGit {
       url = "https://github.com/mozilla/nixpkgs-mozilla.git";
-      rev = "4a07484cf0e49047f82d83fd119acffbad3b235f";
+      rev = "78e723925daf5c9e8d0a1837ec27059e61649cb6";
     });
   nixpkgs = import <nixpkgs> { overlays = [ mozillaOverlay ]; };
   rust-nightly = with nixpkgs; ((rustChannelOf { date = "2022-11-16"; channel = "nightly"; }).rust.override {
     extensions = [ "rust-src" ];
     targets = [ "wasm32-unknown-unknown" ];

Vulnerable code: use num::{BigUint, FromPrimitive, One, ToPrimitive, Zero};
			let gas_cost = calculate_gas_cost(base_len as u64, mod_len as u64, &exponent, &exp_buf);
		assert_eq!(handle.gas_used, 7104); // gas used when ran in geth
      rev = "4a07484cf0e49047f82d83fd119acffbad3b235f";
Fixed code: use num::{BigUint, FromPrimitive, Integer, One, ToPrimitive, Zero};
	mod_is_even: bool,
	.saturating_mul(if mod_is_even { 20 } else { 1 })
			let gas_cost = calculate_gas_cost(
				base_len as u64,
				mod_len as u64,
				&exponent,
				&exp_buf,
				modulus.is_even(),
			);
		assert_eq!(handle.gas_used, 7104 * 20); // gas used when ran in geth (x20)
      rev = "78e723925daf5c9e8d0a1837ec27059e61649cb6";