I256 SHR can overflow usize

[Jon-Becker]

Version
ethers = "1.0.0"

but also occurs on all more recent versions.

Platform
Darwin macbook 21.3.0 Darwin Kernel Version 21.3.0: Wed Jan 5 21:37:58 PST 2022; root:xnu-8019.80.24~20/RELEASE_ARM64_T8101 arm64

Note that this issue can only be reproduced on certain machines.

Description
Essentially, calling

ethers_core::types::i256::I256
fn shr(self, rhs: I256) -> Self::Output

overflowed usize and throws:

I tried this code:

b.value.shr(a.value)

where b.value and a.value are both valid I256 objects. The operation that is causing this issue is

340282366920938463463374607431768190071 >> 0

[dbelv]

So the following code doesn't panic:

let a = I256::from_dec_str("340282366920938463463374607431768190071").unwrap();
let b = I256::zero();
assert_eq!(a >> b, a);
assert_eq!(a.shr(b), a);

Even when messing around, 0 will be able to fit in all types, however this code does panic with,
'Integer overflow when casting to usize', when performing the shr.

let a = I256::from_dec_str("340282366920938463463374607431768190071").unwrap();
let b = I256::zero();
assert_eq!(b >> a, b);
assert_eq!(b.shr(a), b);

I'm assuming this is the error as above.

[Jon-Becker]

Yes my bad. b is 0 and a is 340282366920938463463374607431768190071 in this case.

[DaniPopes]

The actual panic is here in primitive-types, trying to convert the rightside to usize which has to be handled before-hand, after being called in heimdall::VM::_step, while executing a SAR opcode. I256 Shr impl is not an arithmetic shift right, see: https://docs.rs/ethers/latest/ethers/types/struct.I256.html#diversion-from-standard-numeric-types

[dbelv]

Yeah, noticed that the panic was within primitive-types last night due to U256 implementation.

I was going to approach the implementation with checking if we need to actually perform the method (i.e. the rhs of a shift will always fit in a u8 otherwise we know its 0) and then either return the known answer or perform the op.

Although looking at the changes linked seems there was a bit on your mind for clean-up :). I'll leave the implementation decision in your capable hands.

[Jon-Becker]

Thank you guys, appreciate it!