From c6c23ac54539a37e7a81f16d14a3fc0606733f55 Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Thu, 21 Sep 2023 13:26:04 -0400 Subject: [PATCH] Fix(returndatacopy): Data offset out of bounds is still an error even if len==0 (#31) --- core/src/lib.rs | 4 +++- runtime/src/eval/system.rs | 26 ++++++++++++++------------ rust-toolchain.toml | 2 +- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/core/src/lib.rs b/core/src/lib.rs index 49c798ec3..32eab6db9 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs @@ -115,7 +115,9 @@ impl Machine { /// Inspect the machine's next opcode and current stack. #[must_use] pub fn inspect(&self) -> Option<(Opcode, &Stack)> { - let Ok(position) = self.position else { return None }; + let Ok(position) = self.position else { + return None; + }; self.code.get(position).map(|v| (Opcode(*v), &self.stack)) } diff --git a/runtime/src/eval/system.rs b/runtime/src/eval/system.rs index ae5bd3abc..7a1841b1b 100644 --- a/runtime/src/eval/system.rs +++ b/runtime/src/eval/system.rs @@ -148,18 +148,20 @@ pub fn returndatacopy(runtime: &mut Runtime) -> Control { pop_u256!(runtime, data_offset); pop_usize!(runtime, len); - // If `len` is zero then nothing happens, regardless of the - // value of the other parameters. In particular, `memory_offset` - // might be larger than `usize::MAX`, hence why we check this first. - if len == 0 { - return Control::Continue; - } - - // SAFETY: this cast is safe because if `len > 0` then gas cost of memory - // would have already been taken into account at this point. It is impossible - // to have a memory offset greater than `usize::MAX` for any gas limit less - // than `u64::MAX` (and gas limits higher than this are disallowed in general). - let memory_offset = memory_offset.as_usize(); + // If `len` is zero then nothing happens to the memory, regardless + // of the value of `memory_offset`. In particular, the value taken + // from the stack might be larger than `usize::MAX`, hence why the + // `as_usize` cast is not always safe. But because the value does + // not matter when `len == 0` we can safely set it equal to zero instead. + let memory_offset = if len == 0 { + 0 + } else { + // SAFETY: this cast is safe because if `len > 0` then gas cost of memory + // would have already been taken into account at this point. It is impossible + // to have a memory offset greater than `usize::MAX` for any gas limit less + // than `u64::MAX` (and gas limits higher than this are disallowed in general). + memory_offset.as_usize() + }; try_or_fail!(runtime .machine diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 294c8a2ca..052b739c9 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,4 +1,4 @@ [toolchain] -channel = "1.68.2" +channel = "1.72.1" profile = "minimal" components = [ "rustfmt", "clippy" ]