Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt to negate with overflow in disassembler #549

Closed
pcy190 opened this issue Dec 27, 2023 · 0 comments · Fixed by #550
Closed

Attempt to negate with overflow in disassembler #549

pcy190 opened this issue Dec 27, 2023 · 0 comments · Fixed by #550

Comments

@pcy190
Copy link

pcy190 commented Dec 27, 2023

When overflow-check is enabled, the disassembler would panic in signed_off_str when it tries to negate the 0x8000i16 value in

rbpf/src/disassembler.rs

Lines 46 to 52 in b503a18

fn signed_off_str(value: i16) -> String {
if value < 0 {
format!("-{:#x}", -value)
} else {
format!("+{value:#x}")
}
}

The text bytes PoC to trigger it:

[0x7b, 0x21, 0x00, 0x80, 0, 0, 0, 0]

The test code to reproduce:

let loader = BuiltinProgram::new_loader(
        Config::default(),
        FunctionRegistry::default(),
    );
let mut executable = Executable::<TestContextObject>::from_text_bytes(
    &[0x7b, 0x21, 0x00, 0x80, 0, 0, 0, 0],
    std::sync::Arc::new(loader),
    solana_rbpf::program::SBPFVersion::V2,
    FunctionRegistry::default(),
).unwrap();
let analysis = Analysis::from_executable(&executable).unwrap();
let mut reasm = Vec::new();
analysis.disassemble(&mut reasm).unwrap();

Running it in debug mode would encounter:

attempt to negate with overflow
thread 'test_store_negate' panicked at 'attempt to negate with overflow', src/disassembler.rs:48:27
stack backtrace:
   0: rust_begin_unwind
             at /rustc/*/library/std/src/panicking.rs:593:5
   1: core::panicking::panic_fmt
             at /rustc/*/library/core/src/panicking.rs:67:14
   2: core::panicking::panic
             at /rustc/*/library/core/src/panicking.rs:117:5
   3: solana_rbpf::disassembler::signed_off_str
             at ./src/disassembler.rs:48:27
   4: solana_rbpf::disassembler::st_reg_str
             at ./src/disassembler.rs:82:9
   5: solana_rbpf::disassembler::disassemble_instruction
             at ./src/disassembler.rs:142:58

In release mode, the disassembler result would be stxdw [r1-0x8000], r2, which doesn't necessarily pose the wrong result.

However, to enhance the robustness of the disassembler, the negate logic of i16 could be restructured in signed_off_str function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant