Skip to content

Added bounds checking for REP instructions #3727

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.

Sign up for GitHub

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

Draft
AndreaBissoli wants to merge 1 commit into
base: main
Choose a base branch
from
+93 −42
Draft

Added bounds checking for REP instructions #3727

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 32 additions & 22 deletions crates/libafl_frida/src/asan/asan_rt.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2705,12 +2705,12 @@ impl AsanRuntime {
decoder: InstDecoder,
address: u64,
instr: &Insn,
) -> Option<(u8, X86Register, X86Register, u8, i32)> {
) -> Vec<(u8, X86Register, X86Register, u8, i32)> {
let result = frida_to_cs(decoder, instr);

if let Err(e) = result {
log::error!("{e}");
return None;
return vec![];
}

let cs_instr = result.unwrap();
Expand All @@ -2724,19 +2724,14 @@ impl AsanRuntime {
// put nop into the white-list so that instructions like
// like `nop dword [rax + rax]` does not get caught.
match cs_instr.opcode() {
Opcode::LEA | Opcode::NOP => return None,
Opcode::LEA | Opcode::NOP => return vec![],

_ => (),
}

// This is a TODO! In this case, both the src and the dst are mem operand
// so we would need to return two operadns?
if cs_instr.prefixes.rep_any() {
return None;
}

log::trace!("{:#x} {:#?} {:#?}", address, cs_instr, cs_instr.to_string());

let mut res = vec![];
for operand in operands {
if operand.is_memory() {
// log::trace!("{:#?}", operand);
Expand All @@ -2753,13 +2748,23 @@ impl AsanRuntime {
// println!("{:#?}", (memsz, basereg, indexreg, scale, disp));
log::trace!("ASAN Interesting operand {operand:#?}");
log::trace!("{:#?}", (memsz, basereg, indexreg, scale, disp));
return Some((memsz, basereg, indexreg, scale, disp));
res.push((memsz, basereg, indexreg, scale, disp));

if cs_instr.prefixes.rep_any() && indexreg == X86Register::None {
// if the instruction has a rep prefix and does not have an index
// register, then it accesses memory for a contiguous range of addresses.
// we need to check the last accessed address in the range as well
// target address to check is given by base + (RCX * scale) + disp - width

let width = memsz as i32;
res.push((memsz, basereg, X86Register::Rcx, scale, disp - width));
}
}
} // else {} // perhaps avx instructions?
}
}

None
res
}

// FIXME: later for x86
Expand All @@ -2771,12 +2776,12 @@ impl AsanRuntime {
decoder: InstDecoder,
address: u64,
instr: &Insn,
) -> Option<(u8, X86Register, X86Register, u8, i32)> {
) -> Vec<(u8, X86Register, X86Register, u8, i32)> {
let result = frida_to_cs(decoder, instr);

if let Err(e) = result {
log::error!("{e}");
return None;
return vec![];
}

let cs_instr = result.unwrap();
Expand All @@ -2790,19 +2795,14 @@ impl AsanRuntime {
// put nop into the white-list so that instructions like
// like `nop dword [rax + rax]` does not get caught.
match cs_instr.opcode() {
Opcode::LEA | Opcode::NOP => return None,
Opcode::LEA | Opcode::NOP => return vec![],

_ => (),
}

// This is a TODO! In this case, both the src and the dst are mem operand
// so we would need to return two operadns?
if cs_instr.prefixes.rep_any() {
return None;
}

log::trace!("{:#x} {:#?} {:#?}", address, cs_instr, cs_instr.to_string());

let mut res = vec![];
for operand in operands {
if operand.is_memory() {
// log::trace!("{:#?}", operand);
Expand All @@ -2819,13 +2819,23 @@ impl AsanRuntime {
// println!("{:#?}", (memsz, basereg, indexreg, scale, disp));
log::trace!("ASAN Interesting operand {operand:#?}");
log::trace!("{:#?}", (memsz, basereg, indexreg, scale, disp));
return Some((memsz, basereg, indexreg, scale, disp));
res.push((memsz, basereg, indexreg, scale, disp));

if cs_instr.prefixes.rep_any() && indexreg == X86Register::None {
// if the instruction has a rep prefix and does not have an index
// register, then it accesses memory for a contiguous range of addresses.
// we need to check the last accessed address in the range as well
// target address to check is given by base + (RCX * scale) + disp - width

let width = memsz as i32;
res.push((memsz, basereg, X86Register::Rcx, scale, disp - width));
}
}
} // else {} // perhaps avx instructions?
}
}

None
res
}

/// Emits a asan shadow byte check.
Expand Down
49 changes: 29 additions & 20 deletions crates/libafl_frida/src/helper.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -692,33 +692,42 @@ where
}
}


#[cfg(any(target_arch = "x86_64", target_arch = "x86"))]
let res = if let Some(_rt) = runtimes.match_first_type_mut::<AsanRuntime>() {
AsanRuntime::asan_is_interesting_instruction(decoder, address, instr)
} else {
vec![]
};

#[cfg(target_arch = "aarch64")]
let res = if let Some(_rt) = runtimes.match_first_type_mut::<AsanRuntime>() {
AsanRuntime::asan_is_interesting_instruction(decoder, address, instr)
} else {
None
};

#[cfg(any(target_arch = "x86_64", target_arch = "x86"))]
if let Some(details) = res
&& let Some(rt) = runtimes.match_first_type_mut::<AsanRuntime>()
{
let start = output.writer().pc();
rt.emit_shadow_check(
address,
output,
instr.bytes().len(),
details.0,
details.1,
details.2,
details.3,
details.4,
);
log::trace!(
"emitted shadow_check for {:x} at {:x}-{:x}",
address,
start,
output.writer().pc()
);
if let Some(rt) = runtimes.match_first_type_mut::<AsanRuntime>() {
for details in res {
let start = output.writer().pc();
rt.emit_shadow_check(
address,
output,
instr.bytes().len(),
details.0,
details.1,
details.2,
details.3,
details.4,
);
log::trace!(
"emitted shadow_check for {:x} at {:x}-{:x}",
address,
start,
output.writer().pc()
);
}
}

#[cfg(target_arch = "aarch64")]
Expand Down
1 change: 1 addition & 0 deletions crates/libafl_frida/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ mod tests {
),
("malloc_heap_uaf_write", Some("heap use-after-free write")),
("malloc_heap_uaf_read", Some("heap use-after-free read")),
("rep_movsb_overflow_check", Some("heap out-of-bounds read")),
(
"heap_oob_memcpy_read",
Some("function arg resulting in bad read"),
Expand Down
31 changes: 31 additions & 0 deletions crates/libafl_frida/test_harness.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,37 @@ EXTERN int heap_oob_memcpy_write_avx(const uint8_t *_data, size_t _size) {
return 0;
}

EXTERN int rep_movsb_overflow_check(const uint8_t *_data, size_t _size) {
(void)_data;
(void)_size;

// source and destination buffers, with the destination buffer being
// smaller than the source buffer to trigger the overflow
const size_t DST_SIZE = 10;
const size_t SRC_SIZE = DST_SIZE + 1;

char *src = new char[SRC_SIZE];
char *dst = new char[DST_SIZE];
memset(src, 'A', SRC_SIZE);
memset(dst, 'B', DST_SIZE);

size_t n = SRC_SIZE;

asm volatile (
"mov %0, %%rcx\n"
"mov %1, %%rsi\n"
"mov %2, %%rdi\n"
"rep movsb\n" // use the rep movsb instruction to overflow dst
:
: "r"(n), "r"(src), "r"(dst)
: "rcx", "rsi", "rdi", "memory"
);

delete[] src;
delete[] dst;
return 0;
}

EXTERN int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
// abort();
(void)data;
Expand Down
Loading