From 3c98fa7974e6e1fed593054158aba1a3d192803e Mon Sep 17 00:00:00 2001 From: Ahmed Ali Date: Wed, 15 Feb 2023 11:39:16 +0100 Subject: [PATCH 01/28] chore: trigger `capacity overflow` and `attempt to add with overflow` panics --- engine-precompiles/src/modexp.rs | 42 ++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index ea6058c09..c61393f22 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -507,4 +507,46 @@ mod tests { ); assert_eq!(Err(ExitError::OutOfGas), res); } + + #[test] + #[should_panic(expected = "attempt to add with overflow")] + fn test_parse_bytes_reverts_with_attempt_to_add_with_overflow() { + // base_len 0 + // exp_len usize::MAX - 95 + // mod_len 0 + let input = "\ + 0000000000000000000000000000000000000000000000000000000000000000\ + 000000000000000000000000000000000000000000000000ffffffffffffffa0\ + 0000000000000000000000000000000000000000000000000000000000000000\ + ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ + ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; + // Gas cost comes out to 18446744073709551615 + let _ = ModExp::::new().run( + &hex::decode(input).unwrap(), + Some(EthGas::new(100_000)), + &new_context(), + false, + ); + } + + #[test] + #[should_panic(expected = "capacity overflow")] + fn test_parse_bytes_reverts_with_capacity_overflow() { + // base_len 0 + // exp_len usize::MAX - 96 + // mod_len 0 + let input = "\ + 0000000000000000000000000000000000000000000000000000000000000000\ + 000000000000000000000000000000000000000000000000ffffffffffffff9f\ + 0000000000000000000000000000000000000000000000000000000000000000\ + ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ + ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; + // Gas cost comes out to 18446744073709551615 + let _ = ModExp::::new().run( + &hex::decode(input).unwrap(), + Some(EthGas::new(100_000)), + &new_context(), + false, + ); + } } From 73a6d4b4e79cab6b7f31355c61d9d193a8880e66 Mon Sep 17 00:00:00 2001 From: ahmed Date: Wed, 22 Feb 2023 13:20:43 +0100 Subject: [PATCH 02/28] fix: tests and add extra edge cases tests --- engine-precompiles/src/modexp.rs | 92 +++++++++++++++++++++++++++++--- 1 file changed, 85 insertions(+), 7 deletions(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index c61393f22..400ee9998 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -508,19 +508,97 @@ mod tests { assert_eq!(Err(ExitError::OutOfGas), res); } + #[test] + fn test_modexp_without_oom_1() { + // exp_len `isize::MAX` for wasm32 is 0x7fffffff + let input = "\ + 0000000000000000000000000000000000000000000000000000000000000000\ + 000000000000000000000000000000000000000000000000000000007fffffff\ + 0000000000000000000000000000000000000000000000000000000000000000\ + ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ + ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ + ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; + // should pass without panic + let _ = ModExp::::new().run( + &hex::decode(input).unwrap(), + Some(EthGas::new(100_000)), + &new_context(), + false, + ); + } + + #[test] + fn test_modexp_without_oom_2() { + // exp_len `isize::MAX`+ 1 for wasm32 is 0x80000000 + let input = "\ + 0000000000000000000000000000000000000000000000000000000000000000\ + 0000000000000000000000000000000000000000000000000000000080000000\ + 0000000000000000000000000000000000000000000000000000000000000000\ + ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ + ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ + ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; + // should pass without panic! + let _ = ModExp::::new().run( + &hex::decode(input).unwrap(), + Some(EthGas::new(100_000)), + &new_context(), + false, + ); + } + + // #[test] + // #[should_panic()] + // fn test_modexp_oom_1() { + // // this test will panic if exp_len `isize::MAX` = 0x7fffffffffffffff + // let input = "\ + // 0000000000000000000000000000000000000000000000000000000000000000\ + // 0000000000000000000000000000000000000000000000007fffffffffffffff\ + // 0000000000000000000000000000000000000000000000000000000000000000\ + // ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ + // ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ + // ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; + // // should panic with capacity overflow + // let _ = ModExp::::new().run( + // &hex::decode(input).unwrap(), + // Some(EthGas::new(100_000)), + // &new_context(), + // false, + // ); + // } + + // #[test] + // #[should_panic()] + // fn test_modexp_oom_2() { + // // this test will panic if exp_len `isize::MAX` + 1 = 0x8000000000000000 + // let input = "\ + // 0000000000000000000000000000000000000000000000000000000000000000\ + // 0000000000000000000000000000000000000000000000008000000000000000\ + // 0000000000000000000000000000000000000000000000000000000000000000\ + // ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ + // ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ + // ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; + // // should panic with capacity overflow + // let _ = ModExp::::new().run( + // &hex::decode(input).unwrap(), + // Some(EthGas::new(100_000)), + // &new_context(), + // false, + // ); + // } + #[test] #[should_panic(expected = "attempt to add with overflow")] - fn test_parse_bytes_reverts_with_attempt_to_add_with_overflow() { + fn test_modexp_oom_add_with_overflow() { // base_len 0 - // exp_len usize::MAX - 95 + // exp_len usize::MAX 0xffffffffffffffff // mod_len 0 let input = "\ 0000000000000000000000000000000000000000000000000000000000000000\ - 000000000000000000000000000000000000000000000000ffffffffffffffa0\ + 000000000000000000000000000000000000000000000000ffffffffffffffff\ 0000000000000000000000000000000000000000000000000000000000000000\ ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; - // Gas cost comes out to 18446744073709551615 + // should panic let _ = ModExp::::new().run( &hex::decode(input).unwrap(), Some(EthGas::new(100_000)), @@ -531,9 +609,9 @@ mod tests { #[test] #[should_panic(expected = "capacity overflow")] - fn test_parse_bytes_reverts_with_capacity_overflow() { + fn test_modexp_oom_capacity_overflow() { // base_len 0 - // exp_len usize::MAX - 96 + // exp_len usize::MAX - 96 (0xffffffffffffff9f) // mod_len 0 let input = "\ 0000000000000000000000000000000000000000000000000000000000000000\ @@ -541,7 +619,7 @@ mod tests { 0000000000000000000000000000000000000000000000000000000000000000\ ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; - // Gas cost comes out to 18446744073709551615 + // should panic let _ = ModExp::::new().run( &hex::decode(input).unwrap(), Some(EthGas::new(100_000)), From 0fdc2505270c4d4c16e4279de156d8d3da539299 Mon Sep 17 00:00:00 2001 From: ahmed Date: Thu, 9 Mar 2023 12:50:01 +0100 Subject: [PATCH 03/28] fix: clippy error --- engine-precompiles/src/modexp.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index c326221d5..5e1a1ec61 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -520,7 +520,7 @@ mod tests { ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; // should pass without panic - let _ = ModExp::::new().run( + let _res = ModExp::::new().run( &hex::decode(input).unwrap(), Some(EthGas::new(100_000)), &new_context(), @@ -539,7 +539,7 @@ mod tests { ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; // should pass without panic! - let _ = ModExp::::new().run( + let _res = ModExp::::new().run( &hex::decode(input).unwrap(), Some(EthGas::new(100_000)), &new_context(), @@ -600,7 +600,7 @@ mod tests { ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; // should panic - let _ = ModExp::::new().run( + let _res = ModExp::::new().run( &hex::decode(input).unwrap(), Some(EthGas::new(100_000)), &new_context(), @@ -621,7 +621,7 @@ mod tests { ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; // should panic - let _ = ModExp::::new().run( + let _res = ModExp::::new().run( &hex::decode(input).unwrap(), Some(EthGas::new(100_000)), &new_context(), From 45c75cfec84ec587539cd55ffec63682aafb07fa Mon Sep 17 00:00:00 2001 From: ahmed Date: Fri, 17 Mar 2023 11:27:43 +0100 Subject: [PATCH 04/28] fix: update checks for out of memory and capacity errors --- engine-precompiles/src/modexp.rs | 202 +++++++++++++++++-------------- engine-tests/src/tests/repro.rs | 2 +- 2 files changed, 112 insertions(+), 92 deletions(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index 5e1a1ec61..32a017c4c 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -1,11 +1,29 @@ use crate::prelude::types::{Address, EthGas}; use crate::prelude::{PhantomData, Vec, U256}; -use crate::{ - utils, Berlin, Byzantium, EvmPrecompileResult, HardFork, Precompile, PrecompileOutput, -}; +use crate::{Berlin, Byzantium, EvmPrecompileResult, HardFork, Precompile, PrecompileOutput}; +use core::{cmp::min, mem::size_of}; use evm::{Context, ExitError}; use num::{BigUint, Integer}; +// this macro was originaly from https://github.com/bluealloy/revm/blob/8c22748b484ea2eb19d5200cc81ba7a4c3d7f8c7/crates/precompile/src/modexp.rs#L49 +macro_rules! read_u64_with_overflow { + ($input:expr,$from:expr,$to:expr, $overflow_limit:expr) => {{ + const SPLIT: usize = 32 - size_of::(); + let len = $input.len(); + let from_zero = min($from, len); + let from = min(from_zero + SPLIT, len); + let to = min($to, len); + let overflow_bytes = &$input[from_zero..from]; + + let mut len_bytes = [0u8; size_of::()]; + len_bytes[..to - from].copy_from_slice(&$input[from..to]); + #[allow(clippy::as_conversions)] + let out = u64::from_be_bytes(len_bytes) as usize; + let overflow = !(out < $overflow_limit && overflow_bytes.iter().all(|&x| x == 0)); + (out, overflow) + }}; +} + #[derive(Default)] pub struct ModExp(PhantomData); @@ -20,33 +38,27 @@ impl ModExp { impl ModExp { // Note: the output of this function is bounded by 2^67 - fn calc_iter_count(exp_len: u64, base_len: u64, bytes: &[u8]) -> Result { - let start = usize::try_from(base_len).map_err(utils::err_usize_conv)?; - let exp_len = usize::try_from(exp_len).map_err(utils::err_usize_conv)?; + fn calc_iter_count(exp_len: usize, base_len: usize, bytes: &[u8]) -> U256 { let exp = parse_bytes( bytes, - start.saturating_add(96), + base_len.saturating_add(96), core::cmp::min(32, exp_len), // I don't understand why I need a closure here, but doesn't compile without one |x| U256::from(x), ); if exp_len <= 32 && exp.is_zero() { - Ok(U256::zero()) + U256::zero() } else if exp_len <= 32 { - Ok(U256::from(exp.bits()) - U256::from(1)) + U256::from(exp.bits()) - U256::from(1) } else { // else > 32 - Ok(U256::from(8) * U256::from(exp_len - 32) + U256::from(exp.bits()) - U256::from(1)) + U256::from(8) * U256::from(exp_len - 32) + U256::from(exp.bits()) - U256::from(1) } } fn run_inner(input: &[u8]) -> Result, ExitError> { - let (base_len, exp_len, mod_len) = parse_lengths(input); - let base_len = usize::try_from(base_len).map_err(utils::err_usize_conv)?; - let exp_len = usize::try_from(exp_len).map_err(utils::err_usize_conv)?; - let mod_len = usize::try_from(mod_len).map_err(utils::err_usize_conv)?; - + let (base_len, exp_len, mod_len) = parse_lengths(input)?; let base_start = 96; let base_end = base_len.saturating_add(base_start); @@ -84,7 +96,7 @@ impl ModExp { impl ModExp { // ouput of this function is bounded by 2^128 - fn mul_complexity(x: u64) -> U256 { + fn mul_complexity(x: usize) -> U256 { if x <= 64 { U256::from(x * x) } else if x <= 1_024 { @@ -100,10 +112,9 @@ impl ModExp { impl Precompile for ModExp { fn required_gas(input: &[u8]) -> Result { - let (base_len, exp_len, mod_len) = parse_lengths(input); - + let (base_len, exp_len, mod_len) = parse_lengths(input)?; let mul = Self::mul_complexity(core::cmp::max(mod_len, base_len)); - let iter_count = Self::calc_iter_count(exp_len, base_len, input)?; + let iter_count = Self::calc_iter_count(exp_len, base_len, input); // mul * iter_count bounded by 2^195 < 2^256 (no overflow) let gas = mul * core::cmp::max(iter_count, U256::one()) / U256::from(20); @@ -133,7 +144,7 @@ impl Precompile for ModExp { impl ModExp { // output bounded by 2^122 - fn mul_complexity(base_len: u64, mod_len: u64) -> U256 { + fn mul_complexity(base_len: usize, mod_len: usize) -> U256 { let max_len = core::cmp::max(mod_len, base_len); let words = U256::from(Integer::div_ceil(&max_len, &8)); words * words @@ -142,10 +153,9 @@ impl ModExp { impl Precompile for ModExp { fn required_gas(input: &[u8]) -> Result { - let (base_len, exp_len, mod_len) = parse_lengths(input); - + let (base_len, exp_len, mod_len) = parse_lengths(input)?; let mul = Self::mul_complexity(base_len, mod_len); - let iter_count = Self::calc_iter_count(exp_len, base_len, input)?; + let iter_count = Self::calc_iter_count(exp_len, base_len, input); // mul * iter_count bounded by 2^189 (so no overflow) let gas = mul * iter_count.max(U256::one()) / U256::from(3); @@ -199,16 +209,19 @@ fn saturating_round(x: U256) -> u64 { } } -fn parse_lengths(input: &[u8]) -> (u64, u64, u64) { - let parse = |start: usize| -> u64 { - // I don't understand why I need a closure here, but doesn't compile without one - saturating_round(parse_bytes(input, start, 32, |x| U256::from(x))) - }; - let base_len = parse(0); - let exp_len = parse(32); - let mod_len = parse(64); +fn parse_lengths(input: &[u8]) -> Result<(usize, usize, usize), ExitError> { + #[allow(clippy::as_conversions)] + let (base_len, _base_overflow) = read_u64_with_overflow!(input, 0, 32, u32::MAX as usize); + #[allow(clippy::as_conversions)] + let (exp_len, exp_overflow) = read_u64_with_overflow!(input, 32, 64, u32::MAX as usize); + #[allow(clippy::as_conversions)] + let (mod_len, _mod_overflow) = read_u64_with_overflow!(input, 64, 96, u32::MAX as usize); - (base_len, exp_len, mod_len) + if exp_overflow { + return Err(ExitError::OutOfGas); + } + + Ok((base_len, exp_len, mod_len)) } #[cfg(test)] @@ -452,10 +465,11 @@ mod tests { #[test] fn test_berlin_modexp_big_input() { let base_len = U256::from(4); - let exp_len = U256::from(u64::MAX); + let exp_len = U256::from(u32::MAX - 97); let mod_len = U256::from(4); - let base: u32 = 1; + let base: u32 = 0; let exp = U256::MAX; + let mod_val = U256::MAX; let mut input: Vec = Vec::new(); input.extend_from_slice(&u256_to_arr(&base_len)); @@ -463,6 +477,7 @@ mod tests { input.extend_from_slice(&u256_to_arr(&mod_len)); input.extend_from_slice(&base.to_be_bytes()); input.extend_from_slice(&u256_to_arr(&exp)); + input.extend_from_slice(&u256_to_arr(&mod_val)); // completes without any overflow ModExp::::required_gas(&input).unwrap(); @@ -471,7 +486,7 @@ mod tests { #[test] fn test_berlin_modexp_bigger_input() { let base_len = U256::MAX; - let exp_len = U256::MAX; + let exp_len = U256::from(u32::MAX - 97); let mod_len = U256::MAX; let base: u32 = 1; let exp = U256::MAX; @@ -496,18 +511,18 @@ mod tests { assert_eq!(res.output, expected); } - #[test] - fn test_modexp_gas_revert() { - let input = "000000000000090000000000000000"; - // Gas cost comes out to 18446744073709551615 - let res = ModExp::::new().run( - &hex::decode(input).unwrap(), - Some(EthGas::new(100_000)), - &new_context(), - false, - ); - assert_eq!(Err(ExitError::OutOfGas), res); - } + // #[test] + // fn test_modexp_gas_revert() { + // let input = "000000000000090000000000000000"; + // // Gas cost comes out to 18446744073709551615 + // let res = ModExp::::new().run( + // &hex::decode(input).unwrap(), + // Some(EthGas::new(100_000)), + // &new_context(), + // false, + // ); + // assert_eq!(Err(ExitError::OutOfGas), res); + // } #[test] fn test_modexp_without_oom_1() { @@ -520,12 +535,13 @@ mod tests { ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; // should pass without panic - let _res = ModExp::::new().run( + let res = ModExp::::new().run( &hex::decode(input).unwrap(), Some(EthGas::new(100_000)), &new_context(), false, ); + assert!(res.is_ok()); } #[test] @@ -539,56 +555,58 @@ mod tests { ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; // should pass without panic! - let _res = ModExp::::new().run( + let res = ModExp::::new().run( &hex::decode(input).unwrap(), Some(EthGas::new(100_000)), &new_context(), false, ); + assert!(res.is_ok()); } - // #[test] - // #[should_panic()] - // fn test_modexp_oom_1() { - // // this test will panic if exp_len `isize::MAX` = 0x7fffffffffffffff - // let input = "\ - // 0000000000000000000000000000000000000000000000000000000000000000\ - // 0000000000000000000000000000000000000000000000007fffffffffffffff\ - // 0000000000000000000000000000000000000000000000000000000000000000\ - // ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ - // ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ - // ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; - // // should panic with capacity overflow - // let _ = ModExp::::new().run( - // &hex::decode(input).unwrap(), - // Some(EthGas::new(100_000)), - // &new_context(), - // false, - // ); - // } + #[test] + fn test_modexp_oom_1() { + // this test will panic if exp_len `isize::MAX` = 0x7fffffffffffffff + let input = "\ + 0000000000000000000000000000000000000000000000000000000000000000\ + 0000000000000000000000000000000000000000000000007fffffffffffffff\ + 0000000000000000000000000000000000000000000000000000000000000000\ + ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ + ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ + ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; + // should panic with capacity overflow + let res = ModExp::::new().run( + &hex::decode(input).unwrap(), + Some(EthGas::new(100_000)), + &new_context(), + false, + ); + assert!(res.is_err()); + } - // #[test] - // #[should_panic()] - // fn test_modexp_oom_2() { - // // this test will panic if exp_len `isize::MAX` + 1 = 0x8000000000000000 - // let input = "\ - // 0000000000000000000000000000000000000000000000000000000000000000\ - // 0000000000000000000000000000000000000000000000008000000000000000\ - // 0000000000000000000000000000000000000000000000000000000000000000\ - // ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ - // ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ - // ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; - // // should panic with capacity overflow - // let _ = ModExp::::new().run( - // &hex::decode(input).unwrap(), - // Some(EthGas::new(100_000)), - // &new_context(), - // false, - // ); - // } + #[test] + fn test_modexp_oom_2() { + // this test will panic if exp_len `isize::MAX` + 1 = 0x8000000000000000 + let input = "\ + 0000000000000000000000000000000000000000000000000000000000000000\ + 0000000000000000000000000000000000000000000000008000000000000000\ + 0000000000000000000000000000000000000000000000000000000000000000\ + ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ + ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ + ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; + // should panic with capacity overflow + let res = ModExp::::new().run( + &hex::decode(input).unwrap(), + Some(EthGas::new(100_000)), + &new_context(), + false, + ); + + assert!(res.is_err()); + } #[test] - #[should_panic(expected = "attempt to add with overflow")] + // #[should_panic(expected = "attempt to add with overflow")] fn test_modexp_oom_add_with_overflow() { // base_len 0 // exp_len usize::MAX 0xffffffffffffffff @@ -600,16 +618,17 @@ mod tests { ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; // should panic - let _res = ModExp::::new().run( + let res = ModExp::::new().run( &hex::decode(input).unwrap(), Some(EthGas::new(100_000)), &new_context(), false, ); + assert!(res.is_err()); } #[test] - #[should_panic(expected = "capacity overflow")] + // #[should_panic(expected = "capacity overflow")] fn test_modexp_oom_capacity_overflow() { // base_len 0 // exp_len usize::MAX - 96 (0xffffffffffffff9f) @@ -621,11 +640,12 @@ mod tests { ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; // should panic - let _res = ModExp::::new().run( + let res = ModExp::::new().run( &hex::decode(input).unwrap(), Some(EthGas::new(100_000)), &new_context(), false, ); + assert!(res.is_err()); } } diff --git a/engine-tests/src/tests/repro.rs b/engine-tests/src/tests/repro.rs index 42c327181..99d202c24 100644 --- a/engine-tests/src/tests/repro.rs +++ b/engine-tests/src/tests/repro.rs @@ -89,7 +89,7 @@ fn repro_5bEgfRQ() { block_timestamp: 1_651_073_772_931_594_646, input_path: "src/tests/res/input_5bEgfRQ.hex", evm_gas_used: 6_414_105, - near_gas_used: 657, + near_gas_used: 658, }); } From c0d7dd743e311b4778b202eb008585aae3cbb162 Mon Sep 17 00:00:00 2001 From: ahmed Date: Fri, 17 Mar 2023 11:58:09 +0100 Subject: [PATCH 05/28] fix: revert to old gas estimate --- engine-tests/src/tests/repro.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine-tests/src/tests/repro.rs b/engine-tests/src/tests/repro.rs index 99d202c24..42c327181 100644 --- a/engine-tests/src/tests/repro.rs +++ b/engine-tests/src/tests/repro.rs @@ -89,7 +89,7 @@ fn repro_5bEgfRQ() { block_timestamp: 1_651_073_772_931_594_646, input_path: "src/tests/res/input_5bEgfRQ.hex", evm_gas_used: 6_414_105, - near_gas_used: 658, + near_gas_used: 657, }); } From 7a1b5a788509b7f76a19a2f7ce0a1cc494d78b3c Mon Sep 17 00:00:00 2001 From: ahmed Date: Tue, 21 Mar 2023 12:46:56 +0100 Subject: [PATCH 06/28] fix: minor test refactor --- engine-precompiles/src/modexp.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index 32a017c4c..53ee13308 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -472,12 +472,12 @@ mod tests { let mod_val = U256::MAX; let mut input: Vec = Vec::new(); - input.extend_from_slice(&u256_to_arr(&base_len)); - input.extend_from_slice(&u256_to_arr(&exp_len)); - input.extend_from_slice(&u256_to_arr(&mod_len)); - input.extend_from_slice(&base.to_be_bytes()); - input.extend_from_slice(&u256_to_arr(&exp)); - input.extend_from_slice(&u256_to_arr(&mod_val)); + input.extend(u256_to_arr(&base_len)); + input.extend(u256_to_arr(&exp_len)); + input.extend(u256_to_arr(&mod_len)); + input.extend(base.to_be_bytes()); + input.extend(u256_to_arr(&exp)); + input.extend(u256_to_arr(&mod_val)); // completes without any overflow ModExp::::required_gas(&input).unwrap(); @@ -492,11 +492,11 @@ mod tests { let exp = U256::MAX; let mut input: Vec = Vec::new(); - input.extend_from_slice(&u256_to_arr(&base_len)); - input.extend_from_slice(&u256_to_arr(&exp_len)); - input.extend_from_slice(&u256_to_arr(&mod_len)); - input.extend_from_slice(&base.to_be_bytes()); - input.extend_from_slice(&u256_to_arr(&exp)); + input.extend(u256_to_arr(&base_len)); + input.extend(u256_to_arr(&exp_len)); + input.extend(u256_to_arr(&mod_len)); + input.extend(base.to_be_bytes()); + input.extend(u256_to_arr(&exp)); // completes without any overflow ModExp::::required_gas(&input).unwrap(); From cf97350bea2b065c1b099e50a1c4d3c475828124 Mon Sep 17 00:00:00 2001 From: ahmed Date: Wed, 22 Mar 2023 12:28:35 +0100 Subject: [PATCH 07/28] fix: oom but it fails with capacity overflow --- engine-precompiles/src/modexp.rs | 309 +++++++++++++------------------ 1 file changed, 128 insertions(+), 181 deletions(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index 7ce4a308e..5169be2f2 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -1,29 +1,12 @@ use crate::prelude::types::{Address, EthGas}; use crate::prelude::{PhantomData, Vec, U256}; -use crate::{Berlin, Byzantium, EvmPrecompileResult, HardFork, Precompile, PrecompileOutput}; -use core::{cmp::min, mem::size_of}; +use crate::{ + utils, Berlin, Byzantium, EvmPrecompileResult, HardFork, Precompile, PrecompileOutput, +}; +use aurora_engine_types::Cow; use evm::{Context, ExitError}; use num::{BigUint, Integer}; -// this macro was originaly from https://github.com/bluealloy/revm/blob/8c22748b484ea2eb19d5200cc81ba7a4c3d7f8c7/crates/precompile/src/modexp.rs#L49 -macro_rules! read_u64_with_overflow { - ($input:expr,$from:expr,$to:expr, $overflow_limit:expr) => {{ - const SPLIT: usize = 32 - size_of::(); - let len = $input.len(); - let from_zero = min($from, len); - let from = min(from_zero + SPLIT, len); - let to = min($to, len); - let overflow_bytes = &$input[from_zero..from]; - - let mut len_bytes = [0u8; size_of::()]; - len_bytes[..to - from].copy_from_slice(&$input[from..to]); - #[allow(clippy::as_conversions)] - let out = u64::from_be_bytes(len_bytes) as usize; - let overflow = !(out < $overflow_limit && overflow_bytes.iter().all(|&x| x == 0)); - (out, overflow) - }}; -} - #[derive(Default)] pub struct ModExp(PhantomData); @@ -38,28 +21,39 @@ impl ModExp { impl ModExp { // Note: the output of this function is bounded by 2^67 - fn calc_iter_count(exp_len: usize, base_len: usize, bytes: &[u8]) -> U256 { + fn calc_iter_count(exp_len: u64, base_len: u64, bytes: &[u8]) -> Result { + let start = usize::try_from(base_len).map_err(utils::err_usize_conv)?; + let exp_len = usize::try_from(exp_len).map_err(utils::err_usize_conv)?; let exp = parse_bytes( bytes, - base_len.saturating_add(96), + start.saturating_add(96), core::cmp::min(32, exp_len), // I don't understand why I need a closure here, but doesn't compile without one |x| U256::from(x), ); if exp_len <= 32 && exp.is_zero() { - U256::zero() + Ok(U256::zero()) } else if exp_len <= 32 { - U256::from(exp.bits()) - U256::from(1) + Ok(U256::from(exp.bits()) - U256::from(1)) } else { // else > 32 - U256::from(8) * U256::from(exp_len - 32) + U256::from(exp.bits()) - U256::from(1) + Ok(U256::from(8) * U256::from(exp_len - 32) + U256::from(exp.bits()) - U256::from(1)) } } fn run_inner(input: &[u8]) -> Result, ExitError> { - let (base_len, exp_len, mod_len) = parse_lengths(input)?; + let (base_len, exp_len, mod_len) = parse_lengths(input); + let base_len = usize::try_from(base_len).map_err(utils::err_usize_conv)?; + let exp_len = usize::try_from(exp_len).map_err(utils::err_usize_conv)?; + let mod_len = usize::try_from(mod_len).map_err(utils::err_usize_conv)?; + let base_start = 96; + + if base_len == 0 && mod_len == 0 && exp_len > usize::MAX.saturating_sub(base_start) { + return Err(ExitError::Other(Cow::Borrowed("INVALID_MODEXP_LENGTHS"))) + } + let base_end = base_len.saturating_add(base_start); let exp_start = base_end; @@ -97,7 +91,7 @@ impl ModExp { impl ModExp { const MIN_GAS: EthGas = EthGas::new(0); // ouput of this function is bounded by 2^128 - fn mul_complexity(x: usize) -> U256 { + fn mul_complexity(x: u64) -> U256 { if x <= 64 { U256::from(x * x) } else if x <= 1_024 { @@ -113,12 +107,12 @@ impl ModExp { impl Precompile for ModExp { fn required_gas(input: &[u8]) -> Result { - let (base_len, exp_len, mod_len) = parse_lengths(input)?; + let (base_len, exp_len, mod_len) = parse_lengths(input); if base_len == 0 && mod_len == 0 { Ok(Self::MIN_GAS) } else { let mul = Self::mul_complexity(core::cmp::max(mod_len, base_len)); - let iter_count = Self::calc_iter_count(exp_len, base_len, input); + let iter_count = Self::calc_iter_count(exp_len, base_len, input)?; // mul * iter_count bounded by 2^195 < 2^256 (no overflow) let gas = mul * core::cmp::max(iter_count, U256::one()) / U256::from(20); @@ -150,7 +144,7 @@ impl Precompile for ModExp { impl ModExp { const MIN_GAS: EthGas = EthGas::new(200); // output bounded by 2^122 - fn mul_complexity(base_len: usize, mod_len: usize) -> U256 { + fn mul_complexity(base_len: u64, mod_len: u64) -> U256 { let max_len = core::cmp::max(mod_len, base_len); let words = U256::from(Integer::div_ceil(&max_len, &8)); words * words @@ -159,12 +153,12 @@ impl ModExp { impl Precompile for ModExp { fn required_gas(input: &[u8]) -> Result { - let (base_len, exp_len, mod_len) = parse_lengths(input)?; + let (base_len, exp_len, mod_len) = parse_lengths(input); if base_len == 0 && mod_len == 0 { Ok(Self::MIN_GAS) } else { let mul = Self::mul_complexity(base_len, mod_len); - let iter_count = Self::calc_iter_count(exp_len, base_len, input); + let iter_count = Self::calc_iter_count(exp_len, base_len, input)?; // mul * iter_count bounded by 2^189 (so no overflow) let gas = mul * iter_count.max(U256::one()) / U256::from(3); @@ -219,19 +213,16 @@ fn saturating_round(x: U256) -> u64 { } } -fn parse_lengths(input: &[u8]) -> Result<(usize, usize, usize), ExitError> { - #[allow(clippy::as_conversions)] - let (base_len, _base_overflow) = read_u64_with_overflow!(input, 0, 32, u32::MAX as usize); - #[allow(clippy::as_conversions)] - let (exp_len, exp_overflow) = read_u64_with_overflow!(input, 32, 64, u32::MAX as usize); - #[allow(clippy::as_conversions)] - let (mod_len, _mod_overflow) = read_u64_with_overflow!(input, 64, 96, u32::MAX as usize); - - if exp_overflow { - return Err(ExitError::OutOfGas); - } +fn parse_lengths(input: &[u8]) -> (u64, u64, u64) { + let parse = |start: usize| -> u64 { + // I don't understand why I need a closure here, but doesn't compile without one + saturating_round(parse_bytes(input, start, 32, |x| U256::from(x))) + }; + let base_len = parse(0); + let exp_len = parse(32); + let mod_len = parse(64); - Ok((base_len, exp_len, mod_len)) + (base_len, exp_len, mod_len) } #[cfg(test)] @@ -475,11 +466,10 @@ mod tests { #[test] fn test_berlin_modexp_big_input() { let base_len = U256::from(4); - let exp_len = U256::from(u32::MAX - 97); + let exp_len = U256::from(u64::MAX); let mod_len = U256::from(4); - let base: u32 = 0; + let base: u32 = 1; let exp = U256::MAX; - let mod_val = U256::MAX; let mut input: Vec = Vec::new(); input.extend(u256_to_arr(&base_len)); @@ -487,8 +477,6 @@ mod tests { input.extend(u256_to_arr(&mod_len)); input.extend(base.to_be_bytes()); input.extend(u256_to_arr(&exp)); - input.extend(u256_to_arr(&mod_val)); - // completes without any overflow ModExp::::required_gas(&input).unwrap(); @@ -497,7 +485,7 @@ mod tests { #[test] fn test_berlin_modexp_bigger_input() { let base_len = U256::MAX; - let exp_len = U256::from(u32::MAX - 97); + let exp_len = U256::MAX; let mod_len = U256::MAX; let base: u32 = 1; let exp = U256::MAX; @@ -522,142 +510,17 @@ mod tests { assert_eq!(res.output, expected); } - // #[test] - // fn test_modexp_gas_revert() { - // let input = "000000000000090000000000000000"; - // // Gas cost comes out to 18446744073709551615 - // let res = ModExp::::new().run( - // &hex::decode(input).unwrap(), - // Some(EthGas::new(100_000)), - // &new_context(), - // false, - // ); - // assert_eq!(Err(ExitError::OutOfGas), res); - // } - - #[test] - fn test_modexp_without_oom_1() { - // exp_len `isize::MAX` for wasm32 is 0x7fffffff - let input = "\ - 0000000000000000000000000000000000000000000000000000000000000000\ - 000000000000000000000000000000000000000000000000000000007fffffff\ - 0000000000000000000000000000000000000000000000000000000000000000\ - ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ - ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ - ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; - // should pass without panic - let res = ModExp::::new().run( - &hex::decode(input).unwrap(), - Some(EthGas::new(100_000)), - &new_context(), - false, - ); - assert!(res.is_ok()); - } - - #[test] - fn test_modexp_without_oom_2() { - // exp_len `isize::MAX`+ 1 for wasm32 is 0x80000000 - let input = "\ - 0000000000000000000000000000000000000000000000000000000000000000\ - 0000000000000000000000000000000000000000000000000000000080000000\ - 0000000000000000000000000000000000000000000000000000000000000000\ - ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ - ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ - ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; - // should pass without panic! - let res = ModExp::::new().run( - &hex::decode(input).unwrap(), - Some(EthGas::new(100_000)), - &new_context(), - false, - ); - assert!(res.is_ok()); - } - - #[test] - fn test_modexp_oom_1() { - // this test will panic if exp_len `isize::MAX` = 0x7fffffffffffffff - let input = "\ - 0000000000000000000000000000000000000000000000000000000000000000\ - 0000000000000000000000000000000000000000000000007fffffffffffffff\ - 0000000000000000000000000000000000000000000000000000000000000000\ - ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ - ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ - ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; - // should panic with capacity overflow - let res = ModExp::::new().run( - &hex::decode(input).unwrap(), - Some(EthGas::new(100_000)), - &new_context(), - false, - ); - assert!(res.is_err()); - } - #[test] - fn test_modexp_oom_2() { - // this test will panic if exp_len `isize::MAX` + 1 = 0x8000000000000000 - let input = "\ - 0000000000000000000000000000000000000000000000000000000000000000\ - 0000000000000000000000000000000000000000000000008000000000000000\ - 0000000000000000000000000000000000000000000000000000000000000000\ - ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ - ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ - ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; - // should panic with capacity overflow + fn test_modexp_gas_revert() { + let input = "000000000000090000000000000000"; + // Gas cost comes out to 18446744073709551615 let res = ModExp::::new().run( &hex::decode(input).unwrap(), Some(EthGas::new(100_000)), &new_context(), false, ); - - assert!(res.is_err()); - } - - #[test] - // #[should_panic(expected = "attempt to add with overflow")] - fn test_modexp_oom_add_with_overflow() { - // base_len 0 - // exp_len usize::MAX 0xffffffffffffffff - // mod_len 0 - let input = "\ - 0000000000000000000000000000000000000000000000000000000000000000\ - 000000000000000000000000000000000000000000000000ffffffffffffffff\ - 0000000000000000000000000000000000000000000000000000000000000000\ - ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ - ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; - // should panic - let res = ModExp::::new().run( - &hex::decode(input).unwrap(), - Some(EthGas::new(100_000)), - &new_context(), - false, - ); - assert!(res.is_err()); - } - - #[test] - // #[should_panic(expected = "capacity overflow")] - fn test_modexp_oom_capacity_overflow() { - // base_len 0 - // exp_len usize::MAX - 96 (0xffffffffffffff9f) - // mod_len 0 - let input = "\ - 0000000000000000000000000000000000000000000000000000000000000000\ - 000000000000000000000000000000000000000000000000ffffffffffffff9f\ - 0000000000000000000000000000000000000000000000000000000000000000\ - ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\ - ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; - // should panic - let res = ModExp::::new().run( - &hex::decode(input).unwrap(), - Some(EthGas::new(100_000)), - &new_context(), - false, - ); - assert!(res.is_err()); + assert_eq!(Err(ExitError::OutOfGas), res); } #[test] @@ -735,4 +598,88 @@ mod tests { let min_gas = EthGas::new(200); assert_eq!(gas, min_gas); } -} + + #[test] + fn test_modexp_no_oom_with_isize_max() { + // this test should not panic if exp_len is `isize::MAX` + let base_len = U256::from(0); + let exp_len = U256::from(2147483647isize); // `isize::MAX` + let mod_len = U256::from(0); + let base = U256::MAX; + let exp = U256::MAX; + let modulus = U256::MAX; + + let mut input: Vec = Vec::new(); + input.extend(u256_to_arr(&base_len)); + input.extend(u256_to_arr(&exp_len)); + input.extend(u256_to_arr(&mod_len)); + input.extend(u256_to_arr(&base)); + input.extend(u256_to_arr(&exp)); + input.extend(u256_to_arr(&modulus)); + + // should panic with INVALID_MODEXP_LENGTHS due to out-of-memory error + let res = ModExp::::new().run( + &input, + Some(EthGas::new(100_000)), + &new_context(), + false, + ); + assert!(res.is_ok()) + } + + #[test] + fn test_modexp_capacity_overflow() { + // this test should panic with capacity overflow + let base_len = U256::from(0); + let exp_len = U256::from(18446744073709551615usize - 96); // `usize::MAX` - 96 + let mod_len = U256::from(0); + let base = U256::MAX; + let exp = U256::MAX; + let modulus = U256::MAX; + + let mut input: Vec = Vec::new(); + input.extend(u256_to_arr(&base_len)); + input.extend(u256_to_arr(&exp_len)); + input.extend(u256_to_arr(&mod_len)); + input.extend(u256_to_arr(&base)); + input.extend(u256_to_arr(&exp)); + input.extend(u256_to_arr(&modulus)); + + // should panic with INVALID_MODEXP_LENGTHS due to out-of-memory error + let res = ModExp::::new().run( + &input, + Some(EthGas::new(100_000)), + &new_context(), + false, + ); + assert!(res.is_err()) + } + + #[test] + fn test_modexp_add_to_overflow() { + // this test should panic with capacity overflow + let base_len = U256::from(0); + let exp_len = U256::from(18446744073709551615usize); // `usize::MAX` + let mod_len = U256::from(0); + let base = U256::MAX; + let exp = U256::MAX; + let modulus = U256::MAX; + + let mut input: Vec = Vec::new(); + input.extend(u256_to_arr(&base_len)); + input.extend(u256_to_arr(&exp_len)); + input.extend(u256_to_arr(&mod_len)); + input.extend(u256_to_arr(&base)); + input.extend(u256_to_arr(&exp)); + input.extend(u256_to_arr(&modulus)); + + // should panic with INVALID_MODEXP_LENGTHS due to out-of-memory error + let res = ModExp::::new().run( + &input, + Some(EthGas::new(100_000)), + &new_context(), + false, + ); + assert!(res.is_err()) + } +} \ No newline at end of file From dca1d2c5fd2d51c7ceeb92a13b83b58994121390 Mon Sep 17 00:00:00 2001 From: ahmed Date: Wed, 22 Mar 2023 14:17:14 +0100 Subject: [PATCH 08/28] fix: add capacity error check --- engine-precompiles/src/modexp.rs | 84 ++++++++++++++++---------------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index 5169be2f2..2d3e40825 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -24,13 +24,14 @@ impl ModExp { fn calc_iter_count(exp_len: u64, base_len: u64, bytes: &[u8]) -> Result { let start = usize::try_from(base_len).map_err(utils::err_usize_conv)?; let exp_len = usize::try_from(exp_len).map_err(utils::err_usize_conv)?; + let exp = parse_bytes( bytes, start.saturating_add(96), core::cmp::min(32, exp_len), // I don't understand why I need a closure here, but doesn't compile without one |x| U256::from(x), - ); + )?; if exp_len <= 32 && exp.is_zero() { Ok(U256::zero()) @@ -43,7 +44,7 @@ impl ModExp { } fn run_inner(input: &[u8]) -> Result, ExitError> { - let (base_len, exp_len, mod_len) = parse_lengths(input); + let (base_len, exp_len, mod_len) = parse_lengths(input)?; let base_len = usize::try_from(base_len).map_err(utils::err_usize_conv)?; let exp_len = usize::try_from(exp_len).map_err(utils::err_usize_conv)?; let mod_len = usize::try_from(mod_len).map_err(utils::err_usize_conv)?; @@ -51,7 +52,7 @@ impl ModExp { let base_start = 96; if base_len == 0 && mod_len == 0 && exp_len > usize::MAX.saturating_sub(base_start) { - return Err(ExitError::Other(Cow::Borrowed("INVALID_MODEXP_LENGTHS"))) + return Err(ExitError::Other(Cow::Borrowed("INVALID_MODEXP_LENGTHS"))); } let base_end = base_len.saturating_add(base_start); @@ -61,9 +62,9 @@ impl ModExp { let mod_start = exp_end; - let base = parse_bytes(input, base_start, base_len, BigUint::from_bytes_be); - let exponent = parse_bytes(input, exp_start, exp_len, BigUint::from_bytes_be); - let modulus = parse_bytes(input, mod_start, mod_len, BigUint::from_bytes_be); + let base = parse_bytes(input, base_start, base_len, BigUint::from_bytes_be)?; + let exponent = parse_bytes(input, exp_start, exp_len, BigUint::from_bytes_be)?; + let modulus = parse_bytes(input, mod_start, mod_len, BigUint::from_bytes_be)?; let output = { let computed_result = if modulus == BigUint::from(0u32) { @@ -107,7 +108,7 @@ impl ModExp { impl Precompile for ModExp { fn required_gas(input: &[u8]) -> Result { - let (base_len, exp_len, mod_len) = parse_lengths(input); + let (base_len, exp_len, mod_len) = parse_lengths(input)?; if base_len == 0 && mod_len == 0 { Ok(Self::MIN_GAS) } else { @@ -153,7 +154,7 @@ impl ModExp { impl Precompile for ModExp { fn required_gas(input: &[u8]) -> Result { - let (base_len, exp_len, mod_len) = parse_lengths(input); + let (base_len, exp_len, mod_len) = parse_lengths(input)?; if base_len == 0 && mod_len == 0 { Ok(Self::MIN_GAS) } else { @@ -185,12 +186,21 @@ impl Precompile for ModExp { } } -fn parse_bytes T>(input: &[u8], start: usize, size: usize, f: F) -> T { +fn parse_bytes T>( + input: &[u8], + start: usize, + size: usize, + f: F, +) -> Result { let len = input.len(); if start >= len { - return f(&[]); + return Ok(f(&[])); } let end = start + size; + + if end > usize::MAX.saturating_sub(96) { + return Err(ExitError::Other(Cow::Borrowed("INVALID_LENGTH"))); + } if end > len { // Pad on the right with zeros if input is too short let bytes: Vec = input[start..] @@ -199,9 +209,9 @@ fn parse_bytes T>(input: &[u8], start: usize, size: usize .chain(core::iter::repeat(0u8)) .take(size) .collect(); - f(&bytes) + Ok(f(&bytes)) } else { - f(&input[start..end]) + Ok(f(&input[start..end])) } } @@ -213,16 +223,17 @@ fn saturating_round(x: U256) -> u64 { } } -fn parse_lengths(input: &[u8]) -> (u64, u64, u64) { - let parse = |start: usize| -> u64 { +fn parse_lengths(input: &[u8]) -> Result<(u64, u64, u64), ExitError> { + let parse = |start: usize| -> Result { // I don't understand why I need a closure here, but doesn't compile without one - saturating_round(parse_bytes(input, start, 32, |x| U256::from(x))) + let len = parse_bytes(input, start, 32, |x| U256::from(x))?; + Ok(saturating_round(len)) }; - let base_len = parse(0); - let exp_len = parse(32); - let mod_len = parse(64); + let base_len = parse(0)?; + let exp_len = parse(32)?; + let mod_len = parse(64)?; - (base_len, exp_len, mod_len) + Ok((base_len, exp_len, mod_len)) } #[cfg(test)] @@ -618,13 +629,9 @@ mod tests { input.extend(u256_to_arr(&modulus)); // should panic with INVALID_MODEXP_LENGTHS due to out-of-memory error - let res = ModExp::::new().run( - &input, - Some(EthGas::new(100_000)), - &new_context(), - false, - ); - assert!(res.is_ok()) + let res = + ModExp::::new().run(&input, Some(EthGas::new(100_000)), &new_context(), false); + assert!(res.is_ok()); } #[test] @@ -646,18 +653,15 @@ mod tests { input.extend(u256_to_arr(&modulus)); // should panic with INVALID_MODEXP_LENGTHS due to out-of-memory error - let res = ModExp::::new().run( - &input, - Some(EthGas::new(100_000)), - &new_context(), - false, - ); - assert!(res.is_err()) + let res = + ModExp::::new().run(&input, Some(EthGas::new(100_000)), &new_context(), false); + assert!(res.is_err()); } #[test] fn test_modexp_add_to_overflow() { - // this test should panic with capacity overflow + // This test should avoid panicing with capacity overflow + // and instead panic with an ExitError let base_len = U256::from(0); let exp_len = U256::from(18446744073709551615usize); // `usize::MAX` let mod_len = U256::from(0); @@ -674,12 +678,8 @@ mod tests { input.extend(u256_to_arr(&modulus)); // should panic with INVALID_MODEXP_LENGTHS due to out-of-memory error - let res = ModExp::::new().run( - &input, - Some(EthGas::new(100_000)), - &new_context(), - false, - ); - assert!(res.is_err()) + let res = + ModExp::::new().run(&input, Some(EthGas::new(100_000)), &new_context(), false); + assert!(res.is_err()); } -} \ No newline at end of file +} From 0affec64e26a226d1992af73314babddb32b7050 Mon Sep 17 00:00:00 2001 From: ahmed Date: Thu, 23 Mar 2023 16:50:54 +0100 Subject: [PATCH 09/28] fix: never returns errors from precompiles --- engine-precompiles/src/modexp.rs | 73 +++++++++++++++++--------------- 1 file changed, 39 insertions(+), 34 deletions(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index 2d3e40825..2a38200df 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -3,7 +3,6 @@ use crate::prelude::{PhantomData, Vec, U256}; use crate::{ utils, Berlin, Byzantium, EvmPrecompileResult, HardFork, Precompile, PrecompileOutput, }; -use aurora_engine_types::Cow; use evm::{Context, ExitError}; use num::{BigUint, Integer}; @@ -24,14 +23,13 @@ impl ModExp { fn calc_iter_count(exp_len: u64, base_len: u64, bytes: &[u8]) -> Result { let start = usize::try_from(base_len).map_err(utils::err_usize_conv)?; let exp_len = usize::try_from(exp_len).map_err(utils::err_usize_conv)?; - let exp = parse_bytes( bytes, start.saturating_add(96), core::cmp::min(32, exp_len), // I don't understand why I need a closure here, but doesn't compile without one |x| U256::from(x), - )?; + ); if exp_len <= 32 && exp.is_zero() { Ok(U256::zero()) @@ -44,7 +42,7 @@ impl ModExp { } fn run_inner(input: &[u8]) -> Result, ExitError> { - let (base_len, exp_len, mod_len) = parse_lengths(input)?; + let (base_len, exp_len, mod_len) = parse_lengths(input); let base_len = usize::try_from(base_len).map_err(utils::err_usize_conv)?; let exp_len = usize::try_from(exp_len).map_err(utils::err_usize_conv)?; let mod_len = usize::try_from(mod_len).map_err(utils::err_usize_conv)?; @@ -52,9 +50,8 @@ impl ModExp { let base_start = 96; if base_len == 0 && mod_len == 0 && exp_len > usize::MAX.saturating_sub(base_start) { - return Err(ExitError::Other(Cow::Borrowed("INVALID_MODEXP_LENGTHS"))); + return Ok(Vec::new()); } - let base_end = base_len.saturating_add(base_start); let exp_start = base_end; @@ -62,9 +59,12 @@ impl ModExp { let mod_start = exp_end; - let base = parse_bytes(input, base_start, base_len, BigUint::from_bytes_be)?; - let exponent = parse_bytes(input, exp_start, exp_len, BigUint::from_bytes_be)?; - let modulus = parse_bytes(input, mod_start, mod_len, BigUint::from_bytes_be)?; + let base = parse_bytes(input, base_start, base_len, BigUint::from_bytes_be); + let modulus = parse_bytes(input, mod_start, mod_len, BigUint::from_bytes_be); + if modulus == BigUint::from(0u32) { + return Ok(Vec::new()); + } + let exponent = parse_bytes(input, exp_start, exp_len, BigUint::from_bytes_be); let output = { let computed_result = if modulus == BigUint::from(0u32) { @@ -108,7 +108,7 @@ impl ModExp { impl Precompile for ModExp { fn required_gas(input: &[u8]) -> Result { - let (base_len, exp_len, mod_len) = parse_lengths(input)?; + let (base_len, exp_len, mod_len) = parse_lengths(input); if base_len == 0 && mod_len == 0 { Ok(Self::MIN_GAS) } else { @@ -154,7 +154,7 @@ impl ModExp { impl Precompile for ModExp { fn required_gas(input: &[u8]) -> Result { - let (base_len, exp_len, mod_len) = parse_lengths(input)?; + let (base_len, exp_len, mod_len) = parse_lengths(input); if base_len == 0 && mod_len == 0 { Ok(Self::MIN_GAS) } else { @@ -186,21 +186,12 @@ impl Precompile for ModExp { } } -fn parse_bytes T>( - input: &[u8], - start: usize, - size: usize, - f: F, -) -> Result { +fn parse_bytes T>(input: &[u8], start: usize, size: usize, f: F) -> T { let len = input.len(); if start >= len { - return Ok(f(&[])); + return f(&[]); } let end = start + size; - - if end > usize::MAX.saturating_sub(96) { - return Err(ExitError::Other(Cow::Borrowed("INVALID_LENGTH"))); - } if end > len { // Pad on the right with zeros if input is too short let bytes: Vec = input[start..] @@ -209,9 +200,9 @@ fn parse_bytes T>( .chain(core::iter::repeat(0u8)) .take(size) .collect(); - Ok(f(&bytes)) + f(&bytes) } else { - Ok(f(&input[start..end])) + f(&input[start..end]) } } @@ -223,17 +214,16 @@ fn saturating_round(x: U256) -> u64 { } } -fn parse_lengths(input: &[u8]) -> Result<(u64, u64, u64), ExitError> { - let parse = |start: usize| -> Result { +fn parse_lengths(input: &[u8]) -> (u64, u64, u64) { + let parse = |start: usize| -> u64 { // I don't understand why I need a closure here, but doesn't compile without one - let len = parse_bytes(input, start, 32, |x| U256::from(x))?; - Ok(saturating_round(len)) + saturating_round(parse_bytes(input, start, 32, |x| U256::from(x))) }; - let base_len = parse(0)?; - let exp_len = parse(32)?; - let mod_len = parse(64)?; + let base_len = parse(0); + let exp_len = parse(32); + let mod_len = parse(64); - Ok((base_len, exp_len, mod_len)) + (base_len, exp_len, mod_len) } #[cfg(test)] @@ -610,6 +600,18 @@ mod tests { assert_eq!(gas, min_gas); } + #[test] + fn test_out_of_gas_with_max_exponent() { + let input = hex::decode("0000000000000000000000000010000000000000000000000000000000000000000000000000000000000000000000000000000000000000ffffffffffffff9f0000000000000000000000000000000000000000000000000000000000000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); + let res = ModExp::::new().run( + &input.unwrap(), + Some(EthGas::new(100_000)), + &new_context(), + false, + ); + assert_eq!(Err(ExitError::OutOfGas), res); + } + #[test] fn test_modexp_no_oom_with_isize_max() { // this test should not panic if exp_len is `isize::MAX` @@ -628,6 +630,7 @@ mod tests { input.extend(u256_to_arr(&exp)); input.extend(u256_to_arr(&modulus)); + // assert_eq!(encode(&input), encode(&vec![0u8])); // should panic with INVALID_MODEXP_LENGTHS due to out-of-memory error let res = ModExp::::new().run(&input, Some(EthGas::new(100_000)), &new_context(), false); @@ -655,7 +658,7 @@ mod tests { // should panic with INVALID_MODEXP_LENGTHS due to out-of-memory error let res = ModExp::::new().run(&input, Some(EthGas::new(100_000)), &new_context(), false); - assert!(res.is_err()); + assert!(res.is_ok()); } #[test] @@ -677,9 +680,11 @@ mod tests { input.extend(u256_to_arr(&exp)); input.extend(u256_to_arr(&modulus)); + // assert_eq!(encode(&input), encode(&vec![0u8])); + // should panic with INVALID_MODEXP_LENGTHS due to out-of-memory error let res = ModExp::::new().run(&input, Some(EthGas::new(100_000)), &new_context(), false); - assert!(res.is_err()); + assert!(res.is_ok()); } } From eae6dd4ab961f1446c4f1b884cad56fbe1eed0ad Mon Sep 17 00:00:00 2001 From: ahmed Date: Fri, 24 Mar 2023 08:38:11 +0100 Subject: [PATCH 10/28] chore: clean up and add more tests --- engine-precompiles/src/modexp.rs | 40 ++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index 2a38200df..d5f328bb8 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -61,7 +61,10 @@ impl ModExp { let base = parse_bytes(input, base_start, base_len, BigUint::from_bytes_be); let modulus = parse_bytes(input, mod_start, mod_len, BigUint::from_bytes_be); + + // If the output size is bounded by the size of the modulus, skip parsing the exponent if modulus == BigUint::from(0u32) { + return Ok(Vec::new()); } let exponent = parse_bytes(input, exp_start, exp_len, BigUint::from_bytes_be); @@ -601,7 +604,31 @@ mod tests { } #[test] - fn test_out_of_gas_with_max_exponent() { + fn test_max_exp_zero_base_zero_mod() { + let input = hex::decode("0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000ffffffffffffff9f0000000000000000000000000000000000000000000000000000000000000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); + let res = ModExp::::new().run( + &input.unwrap(), + Some(EthGas::new(100_000)), + &new_context(), + false, + ); + assert!(res.is_ok()); + } + + #[test] + fn test_max_exp_max_base_zero_mod() { + let input = hex::decode("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff000000000000000000000000000000000000000000000000ffffffffffffff9f0000000000000000000000000000000000000000000000000000000000000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); + let res = ModExp::::new().run( + &input.unwrap(), + Some(EthGas::new(100_000)), + &new_context(), + false, + ); + assert_eq!(Err(ExitError::OutOfGas), res); + } + + #[test] + fn test_max_exp_non_zero_base_zero_mod() { let input = hex::decode("0000000000000000000000000010000000000000000000000000000000000000000000000000000000000000000000000000000000000000ffffffffffffff9f0000000000000000000000000000000000000000000000000000000000000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); let res = ModExp::::new().run( &input.unwrap(), @@ -630,8 +657,6 @@ mod tests { input.extend(u256_to_arr(&exp)); input.extend(u256_to_arr(&modulus)); - // assert_eq!(encode(&input), encode(&vec![0u8])); - // should panic with INVALID_MODEXP_LENGTHS due to out-of-memory error let res = ModExp::::new().run(&input, Some(EthGas::new(100_000)), &new_context(), false); assert!(res.is_ok()); @@ -639,7 +664,7 @@ mod tests { #[test] fn test_modexp_capacity_overflow() { - // this test should panic with capacity overflow + // this test should not panic with capacity overflow let base_len = U256::from(0); let exp_len = U256::from(18446744073709551615usize - 96); // `usize::MAX` - 96 let mod_len = U256::from(0); @@ -655,7 +680,6 @@ mod tests { input.extend(u256_to_arr(&exp)); input.extend(u256_to_arr(&modulus)); - // should panic with INVALID_MODEXP_LENGTHS due to out-of-memory error let res = ModExp::::new().run(&input, Some(EthGas::new(100_000)), &new_context(), false); assert!(res.is_ok()); @@ -663,8 +687,7 @@ mod tests { #[test] fn test_modexp_add_to_overflow() { - // This test should avoid panicing with capacity overflow - // and instead panic with an ExitError + // This test should not panicing with capacity overflow let base_len = U256::from(0); let exp_len = U256::from(18446744073709551615usize); // `usize::MAX` let mod_len = U256::from(0); @@ -680,9 +703,6 @@ mod tests { input.extend(u256_to_arr(&exp)); input.extend(u256_to_arr(&modulus)); - // assert_eq!(encode(&input), encode(&vec![0u8])); - - // should panic with INVALID_MODEXP_LENGTHS due to out-of-memory error let res = ModExp::::new().run(&input, Some(EthGas::new(100_000)), &new_context(), false); assert!(res.is_ok()); From 202a90c332f6a8e0b789c777c57baa370eaf972d Mon Sep 17 00:00:00 2001 From: ahmed Date: Fri, 24 Mar 2023 09:14:00 +0100 Subject: [PATCH 11/28] chore: more tests --- engine-precompiles/src/modexp.rs | 37 +++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index d5f328bb8..d176f6893 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -64,7 +64,6 @@ impl ModExp { // If the output size is bounded by the size of the modulus, skip parsing the exponent if modulus == BigUint::from(0u32) { - return Ok(Vec::new()); } let exponent = parse_bytes(input, exp_start, exp_len, BigUint::from_bytes_be); @@ -627,6 +626,30 @@ mod tests { assert_eq!(Err(ExitError::OutOfGas), res); } + #[test] + fn test_max_exp_max_base_non_zero_mod() { + let input = hex::decode("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff000000000000000000000000000000000000000000000000ffffffffffffff9f0000000000000000000000000000000000000000000000000000000000000001ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0000000000000000000000000000000000000000000000000000000000000001"); + let res = ModExp::::new().run( + &input.unwrap(), + Some(EthGas::new(100_000)), + &new_context(), + false, + ); + assert_eq!(Err(ExitError::OutOfGas), res); + } + + #[test] + fn test_max_exp_max_base_max_mod() { + let input = hex::decode("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff000000000000000000000000000000000000000000000000ffffffffffffff9fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); + let res = ModExp::::new().run( + &input.unwrap(), + Some(EthGas::new(100_000)), + &new_context(), + false, + ); + assert_eq!(Err(ExitError::OutOfGas), res); + } + #[test] fn test_max_exp_non_zero_base_zero_mod() { let input = hex::decode("0000000000000000000000000010000000000000000000000000000000000000000000000000000000000000000000000000000000000000ffffffffffffff9f0000000000000000000000000000000000000000000000000000000000000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); @@ -639,6 +662,18 @@ mod tests { assert_eq!(Err(ExitError::OutOfGas), res); } + #[test] + fn test_max_exp_zero_base_non_zero_mod() { + let input = hex::decode("0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000ffffffffffffff9f00000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0000000000000000000000000000000000000000000000000000000000000001"); + let res = ModExp::::new().run( + &input.unwrap(), + Some(EthGas::new(100_000)), + &new_context(), + false, + ); + assert_eq!(Err(ExitError::OutOfGas), res); + } + #[test] fn test_modexp_no_oom_with_isize_max() { // this test should not panic if exp_len is `isize::MAX` From f9ba325391977daf533ca45b7fab30b1079aee39 Mon Sep 17 00:00:00 2001 From: ahmed Date: Fri, 24 Mar 2023 10:31:20 +0100 Subject: [PATCH 12/28] refactor: zero modulus check --- engine-precompiles/src/modexp.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index d176f6893..476333937 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -62,16 +62,11 @@ impl ModExp { let base = parse_bytes(input, base_start, base_len, BigUint::from_bytes_be); let modulus = parse_bytes(input, mod_start, mod_len, BigUint::from_bytes_be); - // If the output size is bounded by the size of the modulus, skip parsing the exponent - if modulus == BigUint::from(0u32) { - return Ok(Vec::new()); - } - let exponent = parse_bytes(input, exp_start, exp_len, BigUint::from_bytes_be); - let output = { let computed_result = if modulus == BigUint::from(0u32) { Vec::new() } else { + let exponent = parse_bytes(input, exp_start, exp_len, BigUint::from_bytes_be); base.modpow(&exponent, &modulus).to_bytes_be() }; // The result must be the same length as the input modulus. From 3a30ff0410442d3aedc2f95224486e3c820ee272 Mon Sep 17 00:00:00 2001 From: ahmed Date: Fri, 24 Mar 2023 11:09:07 +0100 Subject: [PATCH 13/28] fix: remove unncessary check --- engine-precompiles/src/modexp.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index 476333937..ed901a49b 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -49,9 +49,6 @@ impl ModExp { let base_start = 96; - if base_len == 0 && mod_len == 0 && exp_len > usize::MAX.saturating_sub(base_start) { - return Ok(Vec::new()); - } let base_end = base_len.saturating_add(base_start); let exp_start = base_end; From 34c53d6f0faddef1090bb1aa349f666a9daac25b Mon Sep 17 00:00:00 2001 From: Ahmed Ali Date: Fri, 24 Mar 2023 17:34:30 +0100 Subject: [PATCH 14/28] Update engine-precompiles/src/modexp.rs Co-authored-by: Michael Birch --- engine-precompiles/src/modexp.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index ed901a49b..81e21e287 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -693,7 +693,7 @@ mod tests { fn test_modexp_capacity_overflow() { // this test should not panic with capacity overflow let base_len = U256::from(0); - let exp_len = U256::from(18446744073709551615usize - 96); // `usize::MAX` - 96 + let exp_len = U256::from(usize::MAX - 96); let mod_len = U256::from(0); let base = U256::MAX; let exp = U256::MAX; From 18cef85d9aa5b0f663f0a4a52c1f96d68aabde12 Mon Sep 17 00:00:00 2001 From: Ahmed Ali Date: Fri, 24 Mar 2023 17:34:46 +0100 Subject: [PATCH 15/28] Update engine-precompiles/src/modexp.rs Co-authored-by: Michael Birch --- engine-precompiles/src/modexp.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index 81e21e287..02011ce0c 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -716,7 +716,7 @@ mod tests { fn test_modexp_add_to_overflow() { // This test should not panicing with capacity overflow let base_len = U256::from(0); - let exp_len = U256::from(18446744073709551615usize); // `usize::MAX` + let exp_len = U256::from(usize::MAX); let mod_len = U256::from(0); let base = U256::MAX; let exp = U256::MAX; From 2ed42b6b5ed841b03c302f1909fd8c68385b63c6 Mon Sep 17 00:00:00 2001 From: ahmed Date: Mon, 27 Mar 2023 11:25:03 +0200 Subject: [PATCH 16/28] fix: add helper function for test inputs --- engine-precompiles/src/modexp.rs | 175 ++++++++++++++----------------- 1 file changed, 76 insertions(+), 99 deletions(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index 02011ce0c..7de5125ca 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -63,6 +63,8 @@ impl ModExp { let computed_result = if modulus == BigUint::from(0u32) { Vec::new() } else { + // The OOM panic is no longer possible because if the modulus is non-zero + // then the required gas prevents passing a huge exponent. let exponent = parse_bytes(input, exp_start, exp_len, BigUint::from_bytes_be); base.modpow(&exponent, &modulus).to_bytes_be() }; @@ -230,6 +232,24 @@ mod tests { // Byzantium tests: https://github.com/holiman/go-ethereum/blob/master/core/vm/testdata/precompiles/modexp.json // Berlin tests:https://github.com/holiman/go-ethereum/blob/master/core/vm/testdata/precompiles/modexp_eip2565.json + fn generate_modexp_test_input( + base_len: U256, + exp_len: U256, + mod_len: U256, + base: U256, + exp: U256, + modulus: U256, + ) -> Vec { + let mut input: Vec = Vec::new(); + input.extend(u256_to_arr(&base_len)); + input.extend(u256_to_arr(&exp_len)); + input.extend(u256_to_arr(&mod_len)); + input.extend(u256_to_arr(&base)); + input.extend(u256_to_arr(&exp)); + input.extend(u256_to_arr(&modulus)); + input + } + struct Test { input: &'static str, expected: &'static str, @@ -460,38 +480,28 @@ mod tests { #[test] fn test_berlin_modexp_big_input() { - let base_len = U256::from(4); - let exp_len = U256::from(u64::MAX); - let mod_len = U256::from(4); - let base: u32 = 1; - let exp = U256::MAX; - - let mut input: Vec = Vec::new(); - input.extend(u256_to_arr(&base_len)); - input.extend(u256_to_arr(&exp_len)); - input.extend(u256_to_arr(&mod_len)); - input.extend(base.to_be_bytes()); - input.extend(u256_to_arr(&exp)); - + let input = generate_modexp_test_input( + U256::from(4), + U256::from(u64::MAX), // `usize::MAX` - 95 + U256::from(4), + U256::from(1), + U256::MAX, + U256::MAX, + ); // completes without any overflow ModExp::::required_gas(&input).unwrap(); } #[test] fn test_berlin_modexp_bigger_input() { - let base_len = U256::MAX; - let exp_len = U256::MAX; - let mod_len = U256::MAX; - let base: u32 = 1; - let exp = U256::MAX; - - let mut input: Vec = Vec::new(); - input.extend(u256_to_arr(&base_len)); - input.extend(u256_to_arr(&exp_len)); - input.extend(u256_to_arr(&mod_len)); - input.extend(base.to_be_bytes()); - input.extend(u256_to_arr(&exp)); - + let input = generate_modexp_test_input( + U256::MAX, + U256::MAX, // `usize::MAX` - 95 + U256::MAX, + U256::from(1), + U256::MAX, + U256::MAX, + ); // completes without any overflow ModExp::::required_gas(&input).unwrap(); } @@ -520,20 +530,14 @@ mod tests { #[test] fn test_modexp_not_overflow() { - let base_len = U256::from(0); - let exp_len = U256::from(4294967200usize); // `usize::MAX` - 95 - let mod_len = U256::from(0); - let base = U256::MAX; - let exp = U256::MAX; - let modulus = U256::MAX; - - let mut input: Vec = Vec::new(); - input.extend(u256_to_arr(&base_len)); - input.extend(u256_to_arr(&exp_len)); - input.extend(u256_to_arr(&mod_len)); - input.extend(u256_to_arr(&base)); - input.extend(u256_to_arr(&exp)); - input.extend(u256_to_arr(&modulus)); + let input = generate_modexp_test_input( + U256::from(0), + U256::from(4294967200usize), // `usize::MAX` - 95 + U256::from(0), + U256::MAX, + U256::MAX, + U256::MAX, + ); let res = ModExp::::new() .run(&input, Some(EthGas::new(100_000)), &new_context(), false) @@ -558,20 +562,14 @@ mod tests { #[test] fn test_zero_base_len_zero_mod_len() { - let base_len = U256::from(0); - let exp_len = U256::from(1); - let mod_len = U256::from(0); - let base = U256::from(1); - let exp = U256::from(1); - let modulus = U256::from(1); - - let mut input: Vec = Vec::new(); - input.extend(u256_to_arr(&base_len)); - input.extend(u256_to_arr(&exp_len)); - input.extend(u256_to_arr(&mod_len)); - input.extend(u256_to_arr(&base)); - input.extend(u256_to_arr(&exp)); - input.extend(u256_to_arr(&modulus)); + let input = generate_modexp_test_input( + U256::from(0), + U256::from(1), // `isize::MAX` + U256::from(0), + U256::from(1), + U256::from(1), + U256::from(1), + ); let res = ModExp::::new() .run(&input, Some(EthGas::new(100_000)), &new_context(), false) @@ -669,21 +667,14 @@ mod tests { #[test] fn test_modexp_no_oom_with_isize_max() { // this test should not panic if exp_len is `isize::MAX` - let base_len = U256::from(0); - let exp_len = U256::from(2147483647isize); // `isize::MAX` - let mod_len = U256::from(0); - let base = U256::MAX; - let exp = U256::MAX; - let modulus = U256::MAX; - - let mut input: Vec = Vec::new(); - input.extend(u256_to_arr(&base_len)); - input.extend(u256_to_arr(&exp_len)); - input.extend(u256_to_arr(&mod_len)); - input.extend(u256_to_arr(&base)); - input.extend(u256_to_arr(&exp)); - input.extend(u256_to_arr(&modulus)); - + let input = generate_modexp_test_input( + U256::from(0), + U256::from(2147483647isize), // `isize::MAX` + U256::from(0), + U256::MAX, + U256::MAX, + U256::MAX, + ); let res = ModExp::::new().run(&input, Some(EthGas::new(100_000)), &new_context(), false); assert!(res.is_ok()); @@ -692,21 +683,14 @@ mod tests { #[test] fn test_modexp_capacity_overflow() { // this test should not panic with capacity overflow - let base_len = U256::from(0); - let exp_len = U256::from(usize::MAX - 96); - let mod_len = U256::from(0); - let base = U256::MAX; - let exp = U256::MAX; - let modulus = U256::MAX; - - let mut input: Vec = Vec::new(); - input.extend(u256_to_arr(&base_len)); - input.extend(u256_to_arr(&exp_len)); - input.extend(u256_to_arr(&mod_len)); - input.extend(u256_to_arr(&base)); - input.extend(u256_to_arr(&exp)); - input.extend(u256_to_arr(&modulus)); - + let input = generate_modexp_test_input( + U256::from(0), + U256::from(usize::MAX - 96), + U256::from(0), + U256::MAX, + U256::MAX, + U256::MAX, + ); let res = ModExp::::new().run(&input, Some(EthGas::new(100_000)), &new_context(), false); assert!(res.is_ok()); @@ -715,21 +699,14 @@ mod tests { #[test] fn test_modexp_add_to_overflow() { // This test should not panicing with capacity overflow - let base_len = U256::from(0); - let exp_len = U256::from(usize::MAX); - let mod_len = U256::from(0); - let base = U256::MAX; - let exp = U256::MAX; - let modulus = U256::MAX; - - let mut input: Vec = Vec::new(); - input.extend(u256_to_arr(&base_len)); - input.extend(u256_to_arr(&exp_len)); - input.extend(u256_to_arr(&mod_len)); - input.extend(u256_to_arr(&base)); - input.extend(u256_to_arr(&exp)); - input.extend(u256_to_arr(&modulus)); - + let input = generate_modexp_test_input( + U256::from(0), + U256::from(usize::MAX), // `isize::MAX` + U256::from(0), + U256::MAX, + U256::MAX, + U256::MAX, + ); let res = ModExp::::new().run(&input, Some(EthGas::new(100_000)), &new_context(), false); assert!(res.is_ok()); From e4ffd830423cb57b34d867171171e2f165c40cdf Mon Sep 17 00:00:00 2001 From: ahmed Date: Mon, 27 Mar 2023 15:59:00 +0200 Subject: [PATCH 17/28] fix: add integration tests --- engine-precompiles/src/modexp.rs | 1 - .../src/benches/res/StandardPrecompiles.sol | 46 ++++++++++++++++--- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index 7de5125ca..e0e9e6d1a 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -48,7 +48,6 @@ impl ModExp { let mod_len = usize::try_from(mod_len).map_err(utils::err_usize_conv)?; let base_start = 96; - let base_end = base_len.saturating_add(base_start); let exp_start = base_end; diff --git a/engine-tests/src/benches/res/StandardPrecompiles.sol b/engine-tests/src/benches/res/StandardPrecompiles.sol index 0d1fe4c97..1122bc03e 100644 --- a/engine-tests/src/benches/res/StandardPrecompiles.sol +++ b/engine-tests/src/benches/res/StandardPrecompiles.sol @@ -52,9 +52,34 @@ contract StandardPrecompiles { // See: https://eips.ethereum.org/EIPS/eip-198 // See: https://etherscan.io/address/0x0000000000000000000000000000000000000005 function test_modexp() public view returns(bool) { - uint256 base = 3; - uint256 exponent = 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2e; - uint256 modulus = 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2f; + // testing overflow + uint256 base_len = 0; + uint256 exponent_len = 0xffffffa0; // `usize::MAX` - 95 + uint256 modulus_len = 0; + uint256 base = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff; + uint256 exponent = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff; + uint256 modulus = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff; + assert(modexp(base_len, exponent_len, modulus_len, base, exponent, modulus) == 0); + // testing out-of-memory 1 + base_len = 0; + exponent_len = 0x80000000; // `isize::MAX` + 1 + modulus_len = 0; + base = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff; + exponent = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff; + modulus = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff; + assert(modexp(base_len, exponent_len, modulus_len, base, exponent, modulus) == 0); + // testing out-of-memory 1 + base_len = 0; + exponent_len = 0x7fffffff; // `isize::MAX` + modulus_len = 0; + base = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff; + exponent = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff; + modulus = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff; + assert(modexp(base_len, exponent_len, modulus_len, base, exponent, modulus) == 0); + // testing happy path + base = 3; + exponent = 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2e; + modulus = 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2f; return modexp(base, exponent, modulus) == 1; } @@ -150,12 +175,19 @@ contract StandardPrecompiles { return output; } - function modexp(uint256 base, uint256 exponent, uint256 modulus) private view returns (uint256 output) { + function modexp( + uint256 base_len, + uint256 exponent_len, + uint256 modulus_len, + uint256 base, + uint256 exponent, + uint256 modulus + ) private view returns (uint256 output) { assembly { let ptr := mload(0x40) - mstore(ptr, 32) - mstore(add(ptr, 0x20), 32) - mstore(add(ptr, 0x40), 32) + mstore(ptr, base_len) + mstore(add(ptr, 0x20), exponent_len) + mstore(add(ptr, 0x40), modulus_len) mstore(add(ptr, 0x60), base) mstore(add(ptr, 0x80), exponent) mstore(add(ptr, 0xA0), modulus) From 074e3d8b90b92866df74b696249003edb2d0798f Mon Sep 17 00:00:00 2001 From: Ahmed Ali Date: Mon, 27 Mar 2023 20:41:24 +0200 Subject: [PATCH 18/28] Update engine-precompiles/src/modexp.rs Co-authored-by: Michael Birch --- engine-precompiles/src/modexp.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index e0e9e6d1a..6bf82a525 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -531,7 +531,7 @@ mod tests { fn test_modexp_not_overflow() { let input = generate_modexp_test_input( U256::from(0), - U256::from(4294967200usize), // `usize::MAX` - 95 + U256::from(usize::MAX - 95), U256::from(0), U256::MAX, U256::MAX, From 5e64b90885b674a00517a2ecde4f0046e890d3f5 Mon Sep 17 00:00:00 2001 From: Ahmed Ali Date: Mon, 27 Mar 2023 20:41:31 +0200 Subject: [PATCH 19/28] Update engine-precompiles/src/modexp.rs Co-authored-by: Michael Birch --- engine-precompiles/src/modexp.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index 6bf82a525..e159abd62 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -481,7 +481,7 @@ mod tests { fn test_berlin_modexp_big_input() { let input = generate_modexp_test_input( U256::from(4), - U256::from(u64::MAX), // `usize::MAX` - 95 + U256::from(u64::MAX), U256::from(4), U256::from(1), U256::MAX, From 08623e1298cb9f93a1485cc9482eef188832d3ca Mon Sep 17 00:00:00 2001 From: Ahmed Ali Date: Mon, 27 Mar 2023 20:41:39 +0200 Subject: [PATCH 20/28] Update engine-precompiles/src/modexp.rs Co-authored-by: Michael Birch --- engine-precompiles/src/modexp.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index e159abd62..ace188fab 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -495,7 +495,7 @@ mod tests { fn test_berlin_modexp_bigger_input() { let input = generate_modexp_test_input( U256::MAX, - U256::MAX, // `usize::MAX` - 95 + U256::MAX, U256::MAX, U256::from(1), U256::MAX, From 0651421ebf6d809b29bf5df47ed838d8d478f496 Mon Sep 17 00:00:00 2001 From: Ahmed Ali Date: Mon, 27 Mar 2023 20:41:44 +0200 Subject: [PATCH 21/28] Update engine-precompiles/src/modexp.rs Co-authored-by: Michael Birch --- engine-precompiles/src/modexp.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index ace188fab..0c33090f1 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -563,7 +563,7 @@ mod tests { fn test_zero_base_len_zero_mod_len() { let input = generate_modexp_test_input( U256::from(0), - U256::from(1), // `isize::MAX` + U256::from(1), U256::from(0), U256::from(1), U256::from(1), From 986d974b6ad15f4c32639c64ad101870a5ad3f4e Mon Sep 17 00:00:00 2001 From: Ahmed Ali Date: Mon, 27 Mar 2023 20:41:52 +0200 Subject: [PATCH 22/28] Update engine-precompiles/src/modexp.rs Co-authored-by: Michael Birch --- engine-precompiles/src/modexp.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index 0c33090f1..51d73169e 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -668,7 +668,7 @@ mod tests { // this test should not panic if exp_len is `isize::MAX` let input = generate_modexp_test_input( U256::from(0), - U256::from(2147483647isize), // `isize::MAX` + U256::from(isize::MAX), U256::from(0), U256::MAX, U256::MAX, From 305505a8cd29da0b35e4b7f91d5063dc5ca857aa Mon Sep 17 00:00:00 2001 From: Ahmed Ali Date: Mon, 27 Mar 2023 20:41:57 +0200 Subject: [PATCH 23/28] Update engine-precompiles/src/modexp.rs Co-authored-by: Michael Birch --- engine-precompiles/src/modexp.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index 51d73169e..e29119e60 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -700,7 +700,7 @@ mod tests { // This test should not panicing with capacity overflow let input = generate_modexp_test_input( U256::from(0), - U256::from(usize::MAX), // `isize::MAX` + U256::from(usize::MAX), U256::from(0), U256::MAX, U256::MAX, From 27f0fe2fd772ffec3f4656413a3b4954e7d4d313 Mon Sep 17 00:00:00 2001 From: ahmed Date: Tue, 28 Mar 2023 08:26:47 +0200 Subject: [PATCH 24/28] fix: `StandardPrecompiles.sol` compilation errors --- engine-tests/src/benches/res/StandardPrecompiles.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine-tests/src/benches/res/StandardPrecompiles.sol b/engine-tests/src/benches/res/StandardPrecompiles.sol index 1122bc03e..3addcfd55 100644 --- a/engine-tests/src/benches/res/StandardPrecompiles.sol +++ b/engine-tests/src/benches/res/StandardPrecompiles.sol @@ -80,7 +80,7 @@ contract StandardPrecompiles { base = 3; exponent = 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2e; modulus = 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2f; - return modexp(base, exponent, modulus) == 1; + return modexp(32, 32, 32, base, exponent, modulus) == 1; } // See: https://eips.ethereum.org/EIPS/eip-196 From 61af4ca7b8665981847f18a9cbb3ee872c95a2ab Mon Sep 17 00:00:00 2001 From: ahmed Date: Tue, 28 Mar 2023 08:38:48 +0200 Subject: [PATCH 25/28] fix: revert to old implementation of `StandardPrecompiles` --- .../src/benches/res/StandardPrecompiles.sol | 48 ++++--------------- 1 file changed, 8 insertions(+), 40 deletions(-) diff --git a/engine-tests/src/benches/res/StandardPrecompiles.sol b/engine-tests/src/benches/res/StandardPrecompiles.sol index 3addcfd55..0d1fe4c97 100644 --- a/engine-tests/src/benches/res/StandardPrecompiles.sol +++ b/engine-tests/src/benches/res/StandardPrecompiles.sol @@ -52,35 +52,10 @@ contract StandardPrecompiles { // See: https://eips.ethereum.org/EIPS/eip-198 // See: https://etherscan.io/address/0x0000000000000000000000000000000000000005 function test_modexp() public view returns(bool) { - // testing overflow - uint256 base_len = 0; - uint256 exponent_len = 0xffffffa0; // `usize::MAX` - 95 - uint256 modulus_len = 0; - uint256 base = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff; - uint256 exponent = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff; - uint256 modulus = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff; - assert(modexp(base_len, exponent_len, modulus_len, base, exponent, modulus) == 0); - // testing out-of-memory 1 - base_len = 0; - exponent_len = 0x80000000; // `isize::MAX` + 1 - modulus_len = 0; - base = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff; - exponent = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff; - modulus = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff; - assert(modexp(base_len, exponent_len, modulus_len, base, exponent, modulus) == 0); - // testing out-of-memory 1 - base_len = 0; - exponent_len = 0x7fffffff; // `isize::MAX` - modulus_len = 0; - base = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff; - exponent = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff; - modulus = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff; - assert(modexp(base_len, exponent_len, modulus_len, base, exponent, modulus) == 0); - // testing happy path - base = 3; - exponent = 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2e; - modulus = 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2f; - return modexp(32, 32, 32, base, exponent, modulus) == 1; + uint256 base = 3; + uint256 exponent = 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2e; + uint256 modulus = 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2f; + return modexp(base, exponent, modulus) == 1; } // See: https://eips.ethereum.org/EIPS/eip-196 @@ -175,19 +150,12 @@ contract StandardPrecompiles { return output; } - function modexp( - uint256 base_len, - uint256 exponent_len, - uint256 modulus_len, - uint256 base, - uint256 exponent, - uint256 modulus - ) private view returns (uint256 output) { + function modexp(uint256 base, uint256 exponent, uint256 modulus) private view returns (uint256 output) { assembly { let ptr := mload(0x40) - mstore(ptr, base_len) - mstore(add(ptr, 0x20), exponent_len) - mstore(add(ptr, 0x40), modulus_len) + mstore(ptr, 32) + mstore(add(ptr, 0x20), 32) + mstore(add(ptr, 0x40), 32) mstore(add(ptr, 0x60), base) mstore(add(ptr, 0x80), exponent) mstore(add(ptr, 0xA0), modulus) From 790d6b66f993cd100caf9065efbf3810a930eaeb Mon Sep 17 00:00:00 2001 From: ahmed Date: Tue, 28 Mar 2023 13:24:05 +0200 Subject: [PATCH 26/28] fix: integration tests --- engine-tests/src/tests/mod.rs | 1 + engine-tests/src/tests/modexp.rs | 51 ++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 engine-tests/src/tests/modexp.rs diff --git a/engine-tests/src/tests/mod.rs b/engine-tests/src/tests/mod.rs index 1606e8e6b..a7fbc4a66 100644 --- a/engine-tests/src/tests/mod.rs +++ b/engine-tests/src/tests/mod.rs @@ -9,6 +9,7 @@ pub mod eth_connector; mod ghsa_3p69_m8gg_fwmf; #[cfg(feature = "meta-call")] mod meta_parsing; +pub mod modexp; mod multisender; mod one_inch; mod pausable_precompiles; diff --git a/engine-tests/src/tests/modexp.rs b/engine-tests/src/tests/modexp.rs new file mode 100644 index 000000000..7bf66b48f --- /dev/null +++ b/engine-tests/src/tests/modexp.rs @@ -0,0 +1,51 @@ +use super::sanity::initialize_transfer; +use crate::prelude::Wei; +use crate::prelude::{Address, U256}; +use crate::test_utils::{self, AuroraRunner, Signer}; + +const MODEXP_ADDRESS: Address = aurora_engine_precompiles::make_address(0, 5); + +#[test] +fn test_modexp_oom() { + let (mut runner, mut signer, _) = initialize_transfer(); + + let inputs = [ + "0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007fffffff0000000000000000000000000000000000000000000000000000000000000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", // exp_len: isize::MAX + "000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000ffffffff0000000000000000000000000000000000000000000000000000000000000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", // exp_len: usize::MAX + ]; + + let outputs = [Vec::new(), Vec::new()]; + + for (input, output) in inputs.iter().zip(outputs.iter()) { + check_wasm_modexp( + &mut runner, + &mut signer, + hex::decode(input).unwrap(), + output, + ); + } +} + +fn check_wasm_modexp( + runner: &mut AuroraRunner, + signer: &mut Signer, + input: Vec, + expected_output: &[u8], +) { + let wasm_result = runner + .submit_with_signer(signer, |nonce| { + aurora_engine_transactions::legacy::TransactionLegacy { + nonce, + gas_price: U256::zero(), + gas_limit: u64::MAX.into(), + to: Some(MODEXP_ADDRESS), + value: Wei::zero(), + data: input, + } + }) + .unwrap(); + assert_eq!( + expected_output, + test_utils::unwrap_success_slice(&wasm_result), + ); +} From b7116b8bf139eaadf92b3453244183232de092ba Mon Sep 17 00:00:00 2001 From: ahmed Date: Wed, 29 Mar 2023 11:57:34 +0200 Subject: [PATCH 27/28] refactor: `generate_modexp_test_input` --- engine-precompiles/src/modexp.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index e29119e60..7614b5bca 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -239,14 +239,15 @@ mod tests { exp: U256, modulus: U256, ) -> Vec { - let mut input: Vec = Vec::new(); - input.extend(u256_to_arr(&base_len)); - input.extend(u256_to_arr(&exp_len)); - input.extend(u256_to_arr(&mod_len)); - input.extend(u256_to_arr(&base)); - input.extend(u256_to_arr(&exp)); - input.extend(u256_to_arr(&modulus)); - input + [ + u256_to_arr(&base_len), + u256_to_arr(&exp_len), + u256_to_arr(&mod_len), + u256_to_arr(&base), + u256_to_arr(&exp), + u256_to_arr(&modulus), + ] + .concat() } struct Test { From daa1af1a1c08edbb73b9edc4ed9eb4cecbc08687 Mon Sep 17 00:00:00 2001 From: ahmed Date: Thu, 30 Mar 2023 12:02:19 +0200 Subject: [PATCH 28/28] fix: adding struct for test input --- engine-precompiles/src/modexp.rs | 131 ++++++++++++++++--------------- 1 file changed, 66 insertions(+), 65 deletions(-) diff --git a/engine-precompiles/src/modexp.rs b/engine-precompiles/src/modexp.rs index 7614b5bca..88d5a2969 100644 --- a/engine-precompiles/src/modexp.rs +++ b/engine-precompiles/src/modexp.rs @@ -231,21 +231,23 @@ mod tests { // Byzantium tests: https://github.com/holiman/go-ethereum/blob/master/core/vm/testdata/precompiles/modexp.json // Berlin tests:https://github.com/holiman/go-ethereum/blob/master/core/vm/testdata/precompiles/modexp_eip2565.json - fn generate_modexp_test_input( + struct ModExpTestInput { base_len: U256, exp_len: U256, mod_len: U256, base: U256, exp: U256, modulus: U256, - ) -> Vec { + } + + fn generate_modexp_test_input(test_input: &ModExpTestInput) -> Vec { [ - u256_to_arr(&base_len), - u256_to_arr(&exp_len), - u256_to_arr(&mod_len), - u256_to_arr(&base), - u256_to_arr(&exp), - u256_to_arr(&modulus), + u256_to_arr(&test_input.base_len), + u256_to_arr(&test_input.exp_len), + u256_to_arr(&test_input.mod_len), + u256_to_arr(&test_input.base), + u256_to_arr(&test_input.exp), + u256_to_arr(&test_input.modulus), ] .concat() } @@ -480,28 +482,28 @@ mod tests { #[test] fn test_berlin_modexp_big_input() { - let input = generate_modexp_test_input( - U256::from(4), - U256::from(u64::MAX), - U256::from(4), - U256::from(1), - U256::MAX, - U256::MAX, - ); + let input = generate_modexp_test_input(&ModExpTestInput { + base_len: U256::from(4), + exp_len: U256::from(u64::MAX), + mod_len: U256::from(4), + base: U256::from(1), + exp: U256::MAX, + modulus: U256::MAX, + }); // completes without any overflow ModExp::::required_gas(&input).unwrap(); } #[test] fn test_berlin_modexp_bigger_input() { - let input = generate_modexp_test_input( - U256::MAX, - U256::MAX, - U256::MAX, - U256::from(1), - U256::MAX, - U256::MAX, - ); + let input = generate_modexp_test_input(&ModExpTestInput { + base_len: U256::MAX, + exp_len: U256::MAX, + mod_len: U256::MAX, + base: U256::from(1), + exp: U256::MAX, + modulus: U256::MAX, + }); // completes without any overflow ModExp::::required_gas(&input).unwrap(); } @@ -530,14 +532,14 @@ mod tests { #[test] fn test_modexp_not_overflow() { - let input = generate_modexp_test_input( - U256::from(0), - U256::from(usize::MAX - 95), - U256::from(0), - U256::MAX, - U256::MAX, - U256::MAX, - ); + let input = generate_modexp_test_input(&ModExpTestInput { + base_len: U256::from(0), + exp_len: U256::from(usize::MAX - 95), + mod_len: U256::from(0), + base: U256::MAX, + exp: U256::MAX, + modulus: U256::MAX, + }); let res = ModExp::::new() .run(&input, Some(EthGas::new(100_000)), &new_context(), false) @@ -562,15 +564,14 @@ mod tests { #[test] fn test_zero_base_len_zero_mod_len() { - let input = generate_modexp_test_input( - U256::from(0), - U256::from(1), - U256::from(0), - U256::from(1), - U256::from(1), - U256::from(1), - ); - + let input = generate_modexp_test_input(&ModExpTestInput { + base_len: U256::from(0), + exp_len: U256::from(1), + mod_len: U256::from(0), + base: U256::from(1), + exp: U256::from(1), + modulus: U256::from(1), + }); let res = ModExp::::new() .run(&input, Some(EthGas::new(100_000)), &new_context(), false) .unwrap(); @@ -667,14 +668,14 @@ mod tests { #[test] fn test_modexp_no_oom_with_isize_max() { // this test should not panic if exp_len is `isize::MAX` - let input = generate_modexp_test_input( - U256::from(0), - U256::from(isize::MAX), - U256::from(0), - U256::MAX, - U256::MAX, - U256::MAX, - ); + let input = generate_modexp_test_input(&ModExpTestInput { + base_len: U256::from(0), + exp_len: U256::from(isize::MAX), + mod_len: U256::from(0), + base: U256::MAX, + exp: U256::MAX, + modulus: U256::MAX, + }); let res = ModExp::::new().run(&input, Some(EthGas::new(100_000)), &new_context(), false); assert!(res.is_ok()); @@ -683,14 +684,14 @@ mod tests { #[test] fn test_modexp_capacity_overflow() { // this test should not panic with capacity overflow - let input = generate_modexp_test_input( - U256::from(0), - U256::from(usize::MAX - 96), - U256::from(0), - U256::MAX, - U256::MAX, - U256::MAX, - ); + let input = generate_modexp_test_input(&ModExpTestInput { + base_len: U256::from(0), + exp_len: U256::from(usize::MAX - 96), + mod_len: U256::from(0), + base: U256::MAX, + exp: U256::MAX, + modulus: U256::MAX, + }); let res = ModExp::::new().run(&input, Some(EthGas::new(100_000)), &new_context(), false); assert!(res.is_ok()); @@ -699,14 +700,14 @@ mod tests { #[test] fn test_modexp_add_to_overflow() { // This test should not panicing with capacity overflow - let input = generate_modexp_test_input( - U256::from(0), - U256::from(usize::MAX), - U256::from(0), - U256::MAX, - U256::MAX, - U256::MAX, - ); + let input = generate_modexp_test_input(&ModExpTestInput { + base_len: U256::from(0), + exp_len: U256::from(usize::MAX), + mod_len: U256::from(0), + base: U256::MAX, + exp: U256::MAX, + modulus: U256::MAX, + }); let res = ModExp::::new().run(&input, Some(EthGas::new(100_000)), &new_context(), false); assert!(res.is_ok());