From ac2556a7add66c1a0186f4f7560101407a67cead Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Fri, 24 Feb 2023 21:52:09 -0800 Subject: [PATCH] Fix opt_diff --- fuzz/fuzz_targets/opt_diff.rs | 15 +- src/bin/waffle-util.rs | 2 +- src/interp.rs | 254 +++++++++++++++++++++++----------- 3 files changed, 190 insertions(+), 81 deletions(-) diff --git a/fuzz/fuzz_targets/opt_diff.rs b/fuzz/fuzz_targets/opt_diff.rs index e6146f8..daf1bff 100644 --- a/fuzz/fuzz_targets/opt_diff.rs +++ b/fuzz/fuzz_targets/opt_diff.rs @@ -23,7 +23,13 @@ fuzz_target!( let start = parsed_module.start_func.unwrap(); - let mut orig_ctx = InterpContext::new(&parsed_module); + let mut orig_ctx = match InterpContext::new(&parsed_module) { + Ok(ctx) => ctx, + Err(e) => { + log::trace!("Rejecting due to instantiation error: {:?}", e); + return; + } + }; orig_ctx.fuel = 10000; match orig_ctx.call(&parsed_module, start, &[]) { @@ -32,6 +38,11 @@ fuzz_target!( log::trace!("Rejecting due to timeout in orig"); return; } + InterpResult::Trap => { + // Silently reject. + log::trace!("Rejecting due to trap in orig"); + return; + } InterpResult::Ok(_) => {} ret => panic!("Bad result: {:?}", ret), } @@ -39,7 +50,7 @@ fuzz_target!( let mut opt_module = parsed_module.clone(); opt_module.per_func_body(|body| body.optimize()); - let mut opt_ctx = InterpContext::new(&opt_module); + let mut opt_ctx = InterpContext::new(&opt_module).unwrap(); // Allow a little leeway for opts to not actually optimize. opt_ctx.fuel = 20000; opt_ctx.call(&opt_module, start, &[]).ok().unwrap(); diff --git a/src/bin/waffle-util.rs b/src/bin/waffle-util.rs index 3ce73c1..5f1c7e3 100644 --- a/src/bin/waffle-util.rs +++ b/src/bin/waffle-util.rs @@ -101,7 +101,7 @@ fn main() -> Result<()> { // Ensure all functions are expanded -- this is necessary // for interpretation. module.expand_all_funcs()?; - let mut ctx = InterpContext::new(&module); + let mut ctx = InterpContext::new(&module)?; debug!("Calling start function"); if let Some(start) = module.start_func { ctx.call(&module, start, &[]).ok().unwrap(); diff --git a/src/interp.rs b/src/interp.rs index 1e457f7..b09e689 100644 --- a/src/interp.rs +++ b/src/interp.rs @@ -37,7 +37,7 @@ impl InterpResult { } impl InterpContext { - pub fn new(module: &Module<'_>) -> Self { + pub fn new(module: &Module<'_>) -> anyhow::Result { let mut memories = PerEntity::default(); for (memory, data) in module.memories.entries() { let mut interp_mem = InterpMemory { @@ -45,8 +45,14 @@ impl InterpContext { max_pages: data.maximum_pages.unwrap_or(0x1_0000), }; for segment in &data.segments { - interp_mem.data[segment.offset..(segment.offset + segment.data.len())] - .copy_from_slice(&segment.data[..]); + let end = match segment.offset.checked_add(segment.data.len()) { + Some(end) => end, + None => anyhow::bail!("Data segment offset + length overflows"), + }; + if end > interp_mem.data.len() { + anyhow::bail!("Data segment out of bounds"); + } + interp_mem.data[segment.offset..end].copy_from_slice(&segment.data[..]); } memories[memory] = interp_mem; } @@ -70,12 +76,12 @@ impl InterpContext { }; } - InterpContext { + Ok(InterpContext { memories, tables, globals, fuel: u64::MAX, - } + }) } pub fn call(&mut self, module: &Module<'_>, func: Func, args: &[ConstVal]) -> InterpResult { @@ -939,125 +945,217 @@ pub fn const_eval( (Operator::Nop, []) => Some(ConstVal::None), (Operator::Unreachable, []) => None, - (Operator::I32Load { memory }, [ConstVal::I32(addr)]) => ctx.map(|global| { + (Operator::I32Load { memory }, [ConstVal::I32(addr)]) => ctx.and_then(|global| { let addr = *addr + memory.offset; - ConstVal::I32(read_u32(&global.memories[memory.memory], addr)) + if addr.checked_add(4)? > global.memories[memory.memory].data.len() as u32 { + return None; + } + Some(ConstVal::I32(read_u32( + &global.memories[memory.memory], + addr, + ))) }), - (Operator::I64Load { memory }, [ConstVal::I32(addr)]) => ctx.map(|global| { + (Operator::I64Load { memory }, [ConstVal::I32(addr)]) => ctx.and_then(|global| { let addr = *addr + memory.offset; - ConstVal::I64(read_u64(&global.memories[memory.memory], addr)) + if addr.checked_add(8)? > global.memories[memory.memory].data.len() as u32 { + return None; + } + Some(ConstVal::I64(read_u64( + &global.memories[memory.memory], + addr, + ))) }), - (Operator::F32Load { memory }, [ConstVal::I32(addr)]) => ctx.map(|global| { + (Operator::F32Load { memory }, [ConstVal::I32(addr)]) => ctx.and_then(|global| { let addr = *addr + memory.offset; - ConstVal::F32(read_u32(&global.memories[memory.memory], addr)) + if addr.checked_add(4)? > global.memories[memory.memory].data.len() as u32 { + return None; + } + Some(ConstVal::F32(read_u32( + &global.memories[memory.memory], + addr, + ))) }), - (Operator::F64Load { memory }, [ConstVal::I32(addr)]) => ctx.map(|global| { + (Operator::F64Load { memory }, [ConstVal::I32(addr)]) => ctx.and_then(|global| { let addr = *addr + memory.offset; - ConstVal::F64(read_u64(&global.memories[memory.memory], addr)) + if addr.checked_add(8)? > global.memories[memory.memory].data.len() as u32 { + return None; + } + Some(ConstVal::F64(read_u64( + &global.memories[memory.memory], + addr, + ))) }), - (Operator::I32Load8S { memory }, [ConstVal::I32(addr)]) => ctx.map(|global| { + (Operator::I32Load8S { memory }, [ConstVal::I32(addr)]) => ctx.and_then(|global| { let addr = *addr + memory.offset; - ConstVal::I32(read_u8(&global.memories[memory.memory], addr) as i8 as i32 as u32) + if addr.checked_add(1)? > global.memories[memory.memory].data.len() as u32 { + return None; + } + Some(ConstVal::I32( + read_u8(&global.memories[memory.memory], addr) as i8 as i32 as u32, + )) }), - (Operator::I32Load8U { memory }, [ConstVal::I32(addr)]) => ctx.map(|global| { + (Operator::I32Load8U { memory }, [ConstVal::I32(addr)]) => ctx.and_then(|global| { let addr = *addr + memory.offset; - ConstVal::I32(read_u8(&global.memories[memory.memory], addr) as u32) + if addr.checked_add(4)? > global.memories[memory.memory].data.len() as u32 { + return None; + } + Some(ConstVal::I32( + read_u8(&global.memories[memory.memory], addr) as u32, + )) }), - (Operator::I32Load16S { memory }, [ConstVal::I32(addr)]) => ctx.map(|global| { + (Operator::I32Load16S { memory }, [ConstVal::I32(addr)]) => ctx.and_then(|global| { let addr = *addr + memory.offset; - ConstVal::I32(read_u16(&global.memories[memory.memory], addr) as i16 as i32 as u32) + if addr.checked_add(2)? > global.memories[memory.memory].data.len() as u32 { + return None; + } + Some(ConstVal::I32( + read_u16(&global.memories[memory.memory], addr) as i16 as i32 as u32, + )) }), - (Operator::I32Load16U { memory }, [ConstVal::I32(addr)]) => ctx.map(|global| { + (Operator::I32Load16U { memory }, [ConstVal::I32(addr)]) => ctx.and_then(|global| { let addr = *addr + memory.offset; - ConstVal::I32(read_u16(&global.memories[memory.memory], addr) as u32) + if addr.checked_add(2)? > global.memories[memory.memory].data.len() as u32 { + return None; + } + Some(ConstVal::I32( + read_u16(&global.memories[memory.memory], addr) as u32, + )) }), - (Operator::I64Load8S { memory }, [ConstVal::I32(addr)]) => ctx.map(|global| { + (Operator::I64Load8S { memory }, [ConstVal::I32(addr)]) => ctx.and_then(|global| { let addr = *addr + memory.offset; - ConstVal::I64(read_u8(&global.memories[memory.memory], addr) as i8 as i64 as u64) + if addr.checked_add(1)? > global.memories[memory.memory].data.len() as u32 { + return None; + } + Some(ConstVal::I64( + read_u8(&global.memories[memory.memory], addr) as i8 as i64 as u64, + )) }), - (Operator::I64Load8U { memory }, [ConstVal::I32(addr)]) => ctx.map(|global| { + (Operator::I64Load8U { memory }, [ConstVal::I32(addr)]) => ctx.and_then(|global| { let addr = *addr + memory.offset; - ConstVal::I64(read_u8(&global.memories[memory.memory], addr) as u64) + if addr.checked_add(1)? > global.memories[memory.memory].data.len() as u32 { + return None; + } + Some(ConstVal::I64( + read_u8(&global.memories[memory.memory], addr) as u64, + )) }), - (Operator::I64Load16S { memory }, [ConstVal::I32(addr)]) => ctx.map(|global| { + (Operator::I64Load16S { memory }, [ConstVal::I32(addr)]) => ctx.and_then(|global| { let addr = *addr + memory.offset; - ConstVal::I64(read_u16(&global.memories[memory.memory], addr) as i16 as i64 as u64) + if addr.checked_add(2)? > global.memories[memory.memory].data.len() as u32 { + return None; + } + Some(ConstVal::I64( + read_u16(&global.memories[memory.memory], addr) as i16 as i64 as u64, + )) }), - (Operator::I64Load16U { memory }, [ConstVal::I32(addr)]) => ctx.map(|global| { + (Operator::I64Load16U { memory }, [ConstVal::I32(addr)]) => ctx.and_then(|global| { let addr = *addr + memory.offset; - ConstVal::I64(read_u16(&global.memories[memory.memory], addr) as u64) + if addr.checked_add(2)? > global.memories[memory.memory].data.len() as u32 { + return None; + } + Some(ConstVal::I64( + read_u16(&global.memories[memory.memory], addr) as u64, + )) }), - (Operator::I64Load32S { memory }, [ConstVal::I32(addr)]) => ctx.map(|global| { + (Operator::I64Load32S { memory }, [ConstVal::I32(addr)]) => ctx.and_then(|global| { let addr = *addr + memory.offset; - ConstVal::I64(read_u32(&global.memories[memory.memory], addr) as i32 as i64 as u64) + if addr.checked_add(4)? > global.memories[memory.memory].data.len() as u32 { + return None; + } + Some(ConstVal::I64( + read_u32(&global.memories[memory.memory], addr) as i32 as i64 as u64, + )) }), - (Operator::I64Load32U { memory }, [ConstVal::I32(addr)]) => ctx.map(|global| { + (Operator::I64Load32U { memory }, [ConstVal::I32(addr)]) => ctx.and_then(|global| { let addr = *addr + memory.offset; - ConstVal::I64(read_u32(&global.memories[memory.memory], addr) as u64) + if addr.checked_add(4)? > global.memories[memory.memory].data.len() as u32 { + return None; + } + Some(ConstVal::I64( + read_u32(&global.memories[memory.memory], addr) as u64, + )) }), - (Operator::I32Store { memory }, [ConstVal::I32(addr), ConstVal::I32(data)]) => { - ctx.map(|global| { + (Operator::I32Store { memory }, [ConstVal::I32(addr), ConstVal::I32(data)]) => ctx + .and_then(|global| { let addr = *addr + memory.offset; + if addr.checked_add(4)? > global.memories[memory.memory].data.len() as u32 { + return None; + } write_u32(&mut global.memories[memory.memory], addr, *data); - ConstVal::None - }) - } - (Operator::I64Store { memory }, [ConstVal::I32(addr), ConstVal::I64(data)]) => { - ctx.map(|global| { + Some(ConstVal::None) + }), + (Operator::I64Store { memory }, [ConstVal::I32(addr), ConstVal::I64(data)]) => ctx + .and_then(|global| { let addr = *addr + memory.offset; + if addr.checked_add(8)? > global.memories[memory.memory].data.len() as u32 { + return None; + } write_u64(&mut global.memories[memory.memory], addr, *data); - ConstVal::None - }) - } - (Operator::I32Store8 { memory }, [ConstVal::I32(addr), ConstVal::I32(data)]) => { - ctx.map(|global| { + Some(ConstVal::None) + }), + (Operator::I32Store8 { memory }, [ConstVal::I32(addr), ConstVal::I32(data)]) => ctx + .and_then(|global| { let addr = *addr + memory.offset; + if addr.checked_add(1)? > global.memories[memory.memory].data.len() as u32 { + return None; + } write_u8(&mut global.memories[memory.memory], addr, *data as u8); - ConstVal::None - }) - } - (Operator::I32Store16 { memory }, [ConstVal::I32(addr), ConstVal::I32(data)]) => { - ctx.map(|global| { + Some(ConstVal::None) + }), + (Operator::I32Store16 { memory }, [ConstVal::I32(addr), ConstVal::I32(data)]) => ctx + .and_then(|global| { let addr = *addr + memory.offset; + if addr.checked_add(2)? > global.memories[memory.memory].data.len() as u32 { + return None; + } write_u16(&mut global.memories[memory.memory], addr, *data as u16); - ConstVal::None - }) - } - (Operator::I64Store8 { memory }, [ConstVal::I32(addr), ConstVal::I64(data)]) => { - ctx.map(|global| { + Some(ConstVal::None) + }), + (Operator::I64Store8 { memory }, [ConstVal::I32(addr), ConstVal::I64(data)]) => ctx + .and_then(|global| { let addr = *addr + memory.offset; + if addr.checked_add(1)? > global.memories[memory.memory].data.len() as u32 { + return None; + } write_u8(&mut global.memories[memory.memory], addr, *data as u8); - ConstVal::None - }) - } - (Operator::I64Store16 { memory }, [ConstVal::I32(addr), ConstVal::I64(data)]) => { - ctx.map(|global| { + Some(ConstVal::None) + }), + (Operator::I64Store16 { memory }, [ConstVal::I32(addr), ConstVal::I64(data)]) => ctx + .and_then(|global| { let addr = *addr + memory.offset; + if addr.checked_add(2)? > global.memories[memory.memory].data.len() as u32 { + return None; + } write_u16(&mut global.memories[memory.memory], addr, *data as u16); - ConstVal::None - }) - } - (Operator::I64Store32 { memory }, [ConstVal::I32(addr), ConstVal::I64(data)]) => { - ctx.map(|global| { + Some(ConstVal::None) + }), + (Operator::I64Store32 { memory }, [ConstVal::I32(addr), ConstVal::I64(data)]) => ctx + .and_then(|global| { let addr = *addr + memory.offset; + if addr.checked_add(4)? > global.memories[memory.memory].data.len() as u32 { + return None; + } write_u32(&mut global.memories[memory.memory], addr, *data as u32); - ConstVal::None - }) - } - (Operator::F32Store { memory }, [ConstVal::I32(addr), ConstVal::F32(data)]) => { - ctx.map(|global| { + Some(ConstVal::None) + }), + (Operator::F32Store { memory }, [ConstVal::I32(addr), ConstVal::F32(data)]) => ctx + .and_then(|global| { let addr = *addr + memory.offset; + if addr.checked_add(4)? > global.memories[memory.memory].data.len() as u32 { + return None; + } write_u32(&mut global.memories[memory.memory], addr, *data); - ConstVal::None - }) - } - (Operator::F64Store { memory }, [ConstVal::I32(addr), ConstVal::F64(data)]) => { - ctx.map(|global| { + Some(ConstVal::None) + }), + (Operator::F64Store { memory }, [ConstVal::I32(addr), ConstVal::F64(data)]) => ctx + .and_then(|global| { let addr = *addr + memory.offset; + if addr.checked_add(8)? > global.memories[memory.memory].data.len() as u32 { + return None; + } write_u64(&mut global.memories[memory.memory], addr, *data); - ConstVal::None - }) - } + Some(ConstVal::None) + }), (_, args) if args.iter().any(|&arg| arg == ConstVal::None) => None, (op, args) => unimplemented!( "Undefined operator or arg combination: {:?}, {:?}",