TOB-FUEL-19: Inconsistent casting in SRWQ instruction

[xgreenx]

Description

The input to the SRWQ instruction gets cast in two different ways which results in two different results on 32-bit systems. This is problematic because the value which is validated through has_ownership_range might be different from the value actually used.

Figure 19.1: The (fuel-vm/fuel-vm/src/interpreter/blockchain.rs#971–984)

let mem_range = MemoryRange::new(
    destination_memory_address,
    (Bytes32::LEN as Word).saturating_mul(num_slots), // first variant
)?;
if !ownership_registers.has_ownership_range(&mem_range) {
    return Err(PanicReason::MemoryOwnership.into())
}
// ...
let dest_end = checked_add_word(
    destination_memory_address,
    Bytes32::LEN.saturating_mul(num_slots as usize) as Word, // second variant
)?;

Note, that this is not a severe issue in this case, because overflowing a 32bit usize value consumes a lot of gas. Furthermore, a value larger than 232-1 can not pass the ownership check above because the memory is only 64 megabytes large.
If enough gas is available and the code is executed on a 32bit system, then the first variant would result in 18446744073709551615 and the second 4294967295, if num_slots == 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF.

Recommendations

Short term, calculate the byte length only once by using the first variant (Bytes32::LEN as Word).saturating_mul(num_slots)). The first variant is safer because it is calculated in the u64 space and no downcasting is required.
Long term, consider disallowing the clippy rule as_coversions in the fuel-vm crate. If certain casts should still be allowed, only disallow certain rules like cast_possible_wrap, cast_possible_truncation or cast_possible_wrap.

[xgreenx]

Fixed with https://github.com/FuelLabs/fuel-vm/pull/511/files#diff-294cbb3cc9c0888f5e1e8c2d57dd7a61e7903d1bcb14cc8d864563a3b2cb476d