TOB-FUEL-36: Consensus between 32bit and 64bit systems can fail

[xgreenx]

Description

Consensus between 32bit and 64bit systems might fail due to a truncating cast. In the following figure, b + c is cast to an usize, which can either be 32bit or 64 bit depending on the architecture. Because of the cast the value of bc can differ between systems, which would hinder the fuel network of forming a consensus if 32bit systems are deployed.

Figure 36.1: Cast which leads to different results on 32bit vs 64bit systems. (fuel-vm/fuel-vm/src/interpreter/memory.rs#290–306)

pub(crate) fn load_byte(
    memory: &[u8; MEM_SIZE],
    pc: RegMut<PC>,
    result: &mut Word,
b: Word,
    c: Word,
) -> Result<(), RuntimeError> {
    let bc = b.saturating_add(c) as usize;
    if bc >= VM_MAX_RAM as RegisterId {
        Err(PanicReason::MemoryOverflow.into())
    } else {
        *result = memory[bc] as Word;
inc_pc(pc) }
}

The following unit test panics on a 64bit system but passes on a 32bit system.

Figure 36.2: Unit test which succeeds on 32bit fails on 64bit systems.

let mut memory: Memory<MEM_SIZE> = vec![1u8; MEM_SIZE].try_into().unwrap();
let mut pc = 4;
let mut result = 0;
load_byte(&memory, RegMut::new(&mut pc), &mut result, 0xFFFFFFFF00000000,
0x0).expect("should fail");

Other occurrences of this bug can be found in read_bytes. The next figure shows the test two test vectors.

Figure 36.3: Unit test which shows two test vectors to triggere divergent behavior depending on the system architecture.

// Test vector 1: for read_bytes
load_word(&memory, RegMut::new(&mut pc), &mut result, 0xFFFFFFFF00000000,
0x0).expect("should fail");
// Test vector 2: for get_transaction_field
let input = GTFInput {
    tx: &Script::default(),
    tx_offset: 0,
    pc: RegMut::new(&mut pc),
};
let mut result = 1;
input.get_transaction_field(&mut result, 0xFFFFFFFF00000000, 0x201).expect("should
fail");

Other casts like in memeq are protected by an if-clause which voids such problems because VM_MAX_RAM is lower than 232-1.

Exploit Attack Scenario

An attack deploys a contract which triggers the truncation when casting. If nodes with different architectures have to form a consensus they fail, because the 32bit node would return a success, while the 64bit node would return a panic.

Recommendations

Short term, use usize::try_from (e.g. usize::try_from(b.saturating_add(c)) and return an error if conversion fails. Remove the comment in ecrecover which questions whether the cast may truncate on 32-bit systems. Overflows can not happen here, because M_MAX_RAM is statically configured as 64 * 1024 * 1024.
Long term, disallow the clippy rule cast_possible_truncation, which avoids truncation of large values when casting them.
Also consider restricting the set of supported architectures for Fuel nodes to 64bit. The most notable non-64 bit architectures are PowerPC and WASM.

[xgreenx]

Related FuelLabs/fuel-core#1261