capstone-rust / capstone-rs

high-level Capstone system bindings for Rust
221 stars 78 forks source link

UAF when `clone`ing an `Insn` with a `detail` pointer #121

Closed clubby789 closed 2 years ago

clubby789 commented 2 years ago

While iterating over operands (from the X86InsnDetail binding) I'm experiencing a crash due to an invalid op_type.

The crash occurs specifically in https://github.com/capstone-rust/capstone-rs/blob/fa37a3779b440fe9efe4f30265a714b09ea77076/capstone-rs/src/arch/x86.rs#L30-L42 - op_type has a value of 1633878019 which causes an invalid read from the match's jump table.

If I force my program to skip the offending instruction, it later panics on an out-of-bounds array access: https://github.com/capstone-rust/capstone-rs/blob/fa37a3779b440fe9efe4f30265a714b09ea77076/capstone-rs/src/arch/mod.rs#L671, where the operands array is of size 8 and op_count has a value of 160.

Neither crash occurs when running under Valgrind, so I haven't been able to trace the root cause of the memory corruption.

I'm not sure if this is a bug in capstone-rs, capstone-sys, or Capstone itself, but I'm reporting it here as that's where the crash is occuring for me.

Unfortunately, I haven't been able to create a reliable reproducer, as the crash is only triggered in a large program. However, this snippet demonstrates the configuation I am using and code I am disassembling. The crash occurs at the instruction at offset 0x30: 0x2030: jmp qword ptr [rip + 0x6e4a]

static CODE: &[u8; 112] = b"\xf3\x0f\x1e\xfaH\x83\xec\x08H\x8b\x05Ao\x00\x00H\x85\xc0t\x02\xff\xd0H\x83\xc4\x08\xc3\x00\x00\x00\x00\x00\xff5Jn\x00\x00\xff%Ln\x00\x00\x0f\x1f@\x00\xff%Jn\x00\x00h\x00\x00\x00\x00\xe9\xe0\xff\xff\xff\xff\x15Jn\x00\x00\xff\x15Dn\x00\x00P\xff\x15=n\x00\x00\xff\x157n\x00\x00\xff\x151n\x00\x00\xff\x15+n\x00\x00\xff\x15%n\x00\x00\xff\x15\x1fn\x00";
use capstone::prelude::*;
use capstone::arch;
use capstone::arch::x86::{
    X86Insn, X86InsnDetail, X86OpMem, X86OperandType,
    X86Reg::{X86_REG_EIP, X86_REG_RIP},
};
use capstone::arch::ArchDetail;
use capstone::{prelude::*, Capstone, Insn, RegAccessType};
fn main() {
    let cs = Capstone::new()
        .x86()
        .mode(arch::x86::ArchMode::Mode64)
        .syntax(arch::x86::ArchSyntax::Intel)
        .detail(true)
        .build()
        .expect("Couldn't initialise Capstone");
    let instructions = cs.disasm_all(CODE, 0x2000).unwrap();
    let instructions: Vec<_> = instructions.iter().collect();
    for i in instructions {
        println!("{}", i);
        if i.mnemonic().unwrap().starts_with('j') {
            let detail = cs.insn_detail(i).unwrap();
            if let ArchDetail::X86Detail(detail) = detail.arch_detail() {
                let res = detail.operands().find_map(|op| {
                    if let X86OperandType::Mem(mem) = op.op_type {
                        // We only accept rip-relative addresses
                        if (mem.base().0 as u32 != X86_REG_EIP && mem.base().0 as u32 != X86_REG_RIP)
                            || mem.index() != RegId::INVALID_REG
                        {
                            None
                        } else {
                            Some((mem, op.access.unwrap()))
                        }
                    } else {
                        None
                    }
                });
                dbg!(res);
            }
        }
    }

}
clubby789 commented 2 years ago

The issue seems to be related to the cloneing of instructions. I was iterating over the text sections (using the object crate) like this:

fn disassemble_section<'a>(cs: &'a Capstone, section: Section<'a, 'a, &'a [u8]>) -> Vec<Insn<'a>> {
    cs.disasm_all(section.data().unwrap(), section.address())
        .expect("Disassembly failed")
        .iter()
        .cloned()
        .collect()
}

Refactoring my code to remove the clone so that the original instruction references are used all the way down prevents the crash.

clubby789 commented 2 years ago

Not sure of the root cause, but lifetimes are improperly handled. The clone implementation for Insn just memcpys the data (including the detail pointer) then Drop is called on each instruction, which calls cs_free, which frees the detail pointer

tmfink commented 2 years ago

Thanks for finding the issue! It's not simple creating a safe API over library like capstone.