From 23d64d088996914c3592a650b64a9214c78d44a4 Mon Sep 17 00:00:00 2001 From: LeanSerra <46695152+LeanSerra@users.noreply.github.com> Date: Fri, 27 Dec 2024 17:41:37 -0300 Subject: [PATCH] fix(levm): modexp pass only first 32 bytes to gas_cost, fix get_slice_or_default (#1572) **Motivation** The modexp precompile was failing in some cases because get_slice_or_default was returning None when not enough calldata was passed, there was a bug in the gas_cost calculation, we need to pass to the gas_cost function the first 32 bytes of the exponent we were passing the whole exponent. **Description** - `get_slice_or_default` creates a vec initialized with zeroes with size:`size_to_expand`, then copies byte by byte the calldata until `upper_limit` or `calldata.len()` whichever is lower, - we return an empty vector (this translates to the value 0 when converted to UINT) if the range is invalid. - the gas_cost function now gets the first 32 bytes of the exponent, if `e_size < 32` we pass the whole exponent, the name of the variable in the gas_cost function was changed to reflect this change. --- crates/vm/levm/src/gas_cost.rs | 8 +++---- crates/vm/levm/src/precompiles.rs | 35 +++++++++++++++++-------------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/crates/vm/levm/src/gas_cost.rs b/crates/vm/levm/src/gas_cost.rs index 5a4333dd2..4b39f2ff7 100644 --- a/crates/vm/levm/src/gas_cost.rs +++ b/crates/vm/levm/src/gas_cost.rs @@ -789,7 +789,7 @@ pub fn identity(data_size: usize) -> Result { } pub fn modexp( - exponent: &BigUint, + exponent_first_32_bytes: &BigUint, base_size: usize, exponent_size: usize, modulus_size: usize, @@ -813,8 +813,8 @@ pub fn modexp( let multiplication_complexity = words.checked_pow(2).ok_or(OutOfGasError::GasCostOverflow)?; - let iteration_count = if exponent_size <= 32 && *exponent != BigUint::ZERO { - exponent + let iteration_count = if exponent_size <= 32 && *exponent_first_32_bytes != BigUint::ZERO { + exponent_first_32_bytes .bits() .checked_sub(1) .ok_or(InternalError::ArithmeticOperationUnderflow)? @@ -825,7 +825,7 @@ pub fn modexp( .checked_mul(8) .ok_or(OutOfGasError::GasCostOverflow)?; extra_size - .checked_add(exponent.bits().max(1)) + .checked_add(exponent_first_32_bytes.bits().max(1)) .ok_or(OutOfGasError::GasCostOverflow)? .checked_sub(1) .ok_or(InternalError::ArithmeticOperationUnderflow)? diff --git a/crates/vm/levm/src/precompiles.rs b/crates/vm/levm/src/precompiles.rs index b37eaa5da..5dec61de5 100644 --- a/crates/vm/levm/src/precompiles.rs +++ b/crates/vm/levm/src/precompiles.rs @@ -294,6 +294,9 @@ pub fn modexp( .checked_add(base_limit) .ok_or(InternalError::ArithmeticOperationOverflow)?; + let modulus_limit = m_size + .checked_add(exponent_limit) + .ok_or(InternalError::ArithmeticOperationOverflow)?; // The reason I use unwrap_or_default is to cover the case where calldata does not reach the required // length, so then we should fill the rest with zeros. The same is done in modulus parsing let b = get_slice_or_default(&calldata, 96, base_limit, b_size)?; @@ -302,16 +305,15 @@ pub fn modexp( let e = get_slice_or_default(&calldata, base_limit, exponent_limit, e_size)?; let exponent = BigUint::from_bytes_be(&e); - let m = match calldata.get(exponent_limit..) { - Some(m) => { - let m_extended = fill_with_zeros(&Bytes::from(m.to_vec()), m_size)?; - m_extended.get(..m_size).unwrap_or_default().to_vec() - } - None => Default::default(), - }; + let m = get_slice_or_default(&calldata, exponent_limit, modulus_limit, m_size)?; let modulus = BigUint::from_bytes_be(&m); - let gas_cost = gas_cost::modexp(&exponent, b_size, e_size, m_size)?; + // first 32 bytes of exponent or exponent if e_size < 32 + let bytes_to_take = 32.min(e_size); + // Use of unwrap_or_default because if e == 0 get_slice_or_default returns an empty vec + let exp_first_32 = BigUint::from_bytes_be(e.get(0..bytes_to_take).unwrap_or_default()); + + let gas_cost = gas_cost::modexp(&exp_first_32, b_size, e_size, m_size)?; increase_precompile_consumed_gas(gas_for_call, gas_cost, consumed_gas)?; let result = mod_exp(base, exponent, modulus); @@ -328,16 +330,17 @@ fn get_slice_or_default( upper_limit: usize, size_to_expand: usize, ) -> Result, VMError> { - match calldata.get(lower_limit..upper_limit) { - Some(e) => { - let e_extended = fill_with_zeros(&Bytes::from(e.to_vec()), size_to_expand)?; - Ok(e_extended - .get(..size_to_expand) - .unwrap_or_default() - .to_vec()) + let upper_limit = calldata.len().min(upper_limit); + if let Some(data) = calldata.get(lower_limit..upper_limit) { + if !data.is_empty() { + let mut extended = vec![0u8; size_to_expand]; + for (dest, data) in extended.iter_mut().zip(data.iter()) { + *dest = *data; + } + return Ok(extended); } - None => Ok(Default::default()), } + Ok(Default::default()) } /// I allow this clippy alert because in the code modulus could never be