Skip to content

Commit

Permalink
Fix(returndatacopy): Data offset out of bounds is still an error even…
Browse files Browse the repository at this point in the history
… if len==0 (#31)
  • Loading branch information
birchmd authored Sep 21, 2023
1 parent df6f494 commit c6c23ac
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 14 deletions.
4 changes: 3 additions & 1 deletion core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand Down
26 changes: 14 additions & 12 deletions runtime/src/eval/system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,18 +148,20 @@ pub fn returndatacopy<H: Handler>(runtime: &mut Runtime) -> Control<H> {
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
Expand Down
2 changes: 1 addition & 1 deletion rust-toolchain.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
[toolchain]
channel = "1.68.2"
channel = "1.72.1"
profile = "minimal"
components = [ "rustfmt", "clippy" ]

0 comments on commit c6c23ac

Please sign in to comment.