From 76a79571c1d3332fe508144717b6f7ce8d44afba Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 10 Nov 2020 14:48:18 -0500 Subject: [PATCH 1/7] feat(tests): Add trace consistency tests These require ALSR to be disabled. --- Cargo.lock | 39 ++++++++++--------- Cargo.toml | 2 +- src/main.rs | 6 +++ src/trace.rs | 106 ++++++++++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 129 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ef5db27..5c6d555 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,9 +2,9 @@ # It is not intended for manual editing. [[package]] name = "aho-corasick" -version = "0.7.13" +version = "0.7.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "043164d8ba5c4c3035fec9bbee8647c0261d788f3474306f93bb65901cae0e86" +checksum = "7404febffaa47dac81aa44dba71523c9d069b1bdc50a77db41195149e17f68e5" dependencies = [ "memchr", ] @@ -43,9 +43,9 @@ checksum = "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693" [[package]] name = "cc" -version = "1.0.60" +version = "1.0.62" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ef611cc68ff783f18535d77ddd080185275713d852c4f5cbb6122c462a7a825c" +checksum = "f1770ced377336a88a67c473594ccc14eca6f4559217c34f64aac8f83d641b40" [[package]] name = "cfg-if" @@ -53,6 +53,12 @@ version = "0.1.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822" +[[package]] +name = "cfg-if" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" + [[package]] name = "clap" version = "2.33.3" @@ -131,14 +137,14 @@ version = "0.4.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4fabed175da42fed1fa0746b0ea71f412aa9d35e76e95e59b192c64b9dc2bf8b" dependencies = [ - "cfg-if", + "cfg-if 0.1.10", ] [[package]] name = "memchr" -version = "2.3.3" +version = "2.3.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3728d817d99e5ac407411fa471ff9800a778d88a24685968b36824eaf4bee400" +checksum = "0ee1c47aaa256ecabcaea351eae4a9b01ef39ed810004e298d2511ed284b1525" [[package]] name = "mttn" @@ -164,19 +170,18 @@ checksum = "83450fe6a6142ddd95fb064b746083fc4ef1705fe81f64a64e1d4b39f54a1055" dependencies = [ "bitflags", "cc", - "cfg-if", + "cfg-if 0.1.10", "libc", ] [[package]] name = "nix" version = "0.19.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "85db2feff6bf70ebc3a4793191517d5f0331100a2f10f9bf93b5e5214f32b7b7" +source = "git+https://github.com/trailofbits/nix?branch=ww/personality#ab43430c37eb4106d89e2e3832c99419a45fa598" dependencies = [ "bitflags", "cc", - "cfg-if", + "cfg-if 1.0.0", "libc", ] @@ -200,9 +205,9 @@ dependencies = [ [[package]] name = "regex" -version = "1.3.9" +version = "1.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9c3780fcf44b193bc4d09f36d2a3c87b251da4a046c87795a0d35f4f927ad8e6" +checksum = "38cf2c13ed4745de91a5eb834e11c00bcc3709e773173b2ce4c56c9fbde04b9c" dependencies = [ "aho-corasick", "memchr", @@ -212,9 +217,9 @@ dependencies = [ [[package]] name = "regex-syntax" -version = "0.6.18" +version = "0.6.21" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "26412eb97c6b088a6997e05f69403a802a92d520de2f8e63c2b65f9e0f47c4e8" +checksum = "3b181ba2dcf07aaccad5448e8ead58db5b742cf85dfe035e2227f137a539a189" [[package]] name = "rustc_version" @@ -300,9 +305,9 @@ checksum = "8ea5119cdb4c55b55d432abb513a0429384878c15dde60cc77b1c99de1a95a6a" [[package]] name = "syn" -version = "1.0.44" +version = "1.0.48" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e03e57e4fcbfe7749842d53e24ccb9aa12b7252dbe5e91d2acad31834c8b8fdd" +checksum = "cc371affeffc477f42a221a1e4297aedcea33d47d19b61455588bd9d8f6b19ac" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index 74f7620..4828fa0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,7 +11,7 @@ env_logger = "0.8" iced-x86 = "1.9.1" libc = "0.2" log = "0.4" -nix = "0.19" +nix = { git = "https://github.com/trailofbits/nix", branch = "ww/personality" } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" spawn-ptrace = "0.1.2" diff --git a/src/main.rs b/src/main.rs index 8dfb8f5..b175f30 100644 --- a/src/main.rs +++ b/src/main.rs @@ -30,6 +30,12 @@ fn app<'a, 'b>() -> App<'a, 'b> { .short("d") .long("debug-on-fault"), ) + .arg( + Arg::with_name("disable-aslr") + .help("Disable ASLR on the tracee") + .short("a") + .long("disable-aslr"), + ) .arg( Arg::with_name("tracee-pid") .help("Attach to the given PID for tracing") diff --git a/src/trace.rs b/src/trace.rs index 1ca6b98..a089a80 100644 --- a/src/trace.rs +++ b/src/trace.rs @@ -3,6 +3,7 @@ use iced_x86::{ Code, Decoder, DecoderOptions, Instruction, InstructionInfoFactory, InstructionInfoOptions, MemorySize, Mnemonic, OpAccess, Register, }; +use nix::sys::personality::{self, Persona}; use nix::sys::ptrace; use nix::sys::signal; use nix::sys::uio; @@ -12,16 +13,37 @@ use serde::Serialize; use spawn_ptrace::CommandPtraceSpawn; use std::convert::{TryFrom, TryInto}; +use std::io; +use std::os::unix::process::CommandExt; use std::process::Command; const MAX_INSTR_LEN: usize = 15; +pub trait CommandPersonality { + fn personality(&mut self, persona: Persona); +} + +impl CommandPersonality for Command { + fn personality(&mut self, persona: Persona) { + unsafe { + self.pre_exec(move || match personality::set(persona) { + Ok(_) => Ok(()), + Err(nix::Error::Sys(e)) => Err(io::Error::from_raw_os_error(e as i32)), + Err(_) => Err(io::Error::new( + io::ErrorKind::Other, + "unknown personality error", + )), + }) + }; + } +} + /// Represents the width of a concrete memory operation. /// /// All `mttn` memory operations are 1, 2, 4, or 8 bytes. /// Larger operations are either modeled as multiple individual operations /// (if caused by a `REP` prefix), ignored (if configured), or cause a fatal error. -#[derive(Clone, Copy, Debug, Serialize)] +#[derive(Clone, Copy, Debug, PartialEq, Serialize)] pub enum MemoryMask { Byte = 1, Word = 2, @@ -65,7 +87,7 @@ pub enum MemoryOp { /// Represents an entire traced memory operation, including its kind (`MemoryOp`), /// size (`MemoryMask`), concrete address, and actual read or written data. -#[derive(Debug, Serialize)] +#[derive(Debug, PartialEq, Serialize)] pub struct MemoryHint { address: u64, operation: MemoryOp, @@ -76,7 +98,7 @@ pub struct MemoryHint { /// Represents an individual step in the trace, including the raw instruction bytes, /// the register file state before execution, and any memory operations that result /// from execution. -#[derive(Debug, Serialize)] +#[derive(Debug, PartialEq, Serialize)] pub struct Step { instr: Vec, regs: RegisterFile, @@ -88,7 +110,7 @@ pub struct Step { /// Only the standard addressable registers, plus `RFLAGS`, are tracked. /// `mttn` assumes that all segment base addresses are `0` and therefore doesn't /// track them. -#[derive(Clone, Copy, Debug, Default, Serialize)] +#[derive(Clone, Copy, Debug, Default, PartialEq, Serialize)] pub struct RegisterFile { rax: u64, rbx: u64, @@ -538,6 +560,7 @@ impl Iterator for Tracee<'_> { pub struct Tracer { pub ignore_unsupported_memops: bool, pub debug_on_fault: bool, + pub disable_aslr: bool, pub bitness: u32, pub target: Target, } @@ -560,6 +583,7 @@ impl From> for Tracer { Self { ignore_unsupported_memops: matches.is_present("ignore-unsupported-memops"), debug_on_fault: matches.is_present("debug-on-fault"), + disable_aslr: matches.is_present("disable-aslr"), bitness: matches.value_of("mode").unwrap().parse().unwrap(), target: target, } @@ -570,7 +594,16 @@ impl Tracer { pub fn trace(&self) -> Result { let tracee_pid = match &self.target { Target::Program(name, args) => { - let child = Command::new(name).args(args).spawn_ptrace()?; + let child = { + let mut cmd = Command::new(name); + cmd.args(args); + + if self.disable_aslr { + cmd.personality(Persona::ADDR_NO_RANDOMIZE); + } + + cmd.spawn_ptrace()? + }; log::debug!("spawned {} for tracing as child {}", name, child.id()); @@ -594,7 +627,7 @@ impl Tracer { #[cfg(test)] mod tests { use super::*; - use iced_x86::UsedMemory; + use std::path::PathBuf; fn dummy_regs() -> RegisterFile { RegisterFile { @@ -604,6 +637,41 @@ mod tests { } } + fn build_test_program(program: &str) -> String { + let mut buf = { + let mut buf = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + buf.push("test"); + + buf + }; + + let status = Command::new("make") + .arg("-C") + .arg(&buf) + .arg(program) + .status() + .expect(&format!("build failed: {}", program)); + + if !status.success() { + panic!("build failed: {}", program); + } + + buf.push(program); + buf.into_os_string().into_string().unwrap() + } + + fn test_program_tracer(program: &str) -> Tracer { + let target = Target::Program(program.into(), vec![]); + + Tracer { + ignore_unsupported_memops: false, + debug_on_fault: false, + disable_aslr: true, + bitness: 32, + target: target, + } + } + #[test] fn test_register_file_value() { let regs = dummy_regs(); @@ -632,4 +700,30 @@ mod tests { // Unaddressable and unsupported registers return an Err. assert!(regs.value(Register::ST0).is_err()); } + + #[test] + fn test_trace_consistency() { + for program in &["memops.elf", "stosb.elf", "stosw.elf", "stosd.elf"] { + let program = build_test_program(program); + let tracer = test_program_tracer(&program); + + // TODO(ww): Don't collect these. + let trace1 = tracer + .trace() + .expect("spawn failed") + .collect::>>() + .expect("trace failed"); + + let trace2 = tracer + .trace() + .expect("spawn failed") + .collect::>>() + .expect("trace failed"); + + assert_eq!(trace1, trace2); + for (step1, step2) in trace1.iter().zip(trace2.iter()) { + assert_eq!(step1, step2); + } + } + } } From ad7807d05f13db741812bb46e50c02446e5fe0f2 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 10 Nov 2020 14:57:28 -0500 Subject: [PATCH 2/7] workflows/ci: Install nasm for tests --- .github/workflows/ci.yml | 3 +++ Cargo.lock | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5b0e460..db116e3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,6 +26,9 @@ jobs: steps: - uses: actions/checkout@v2 + - name: System deps + run: apt install -y nasm + - name: Build run: cargo build diff --git a/Cargo.lock b/Cargo.lock index 5c6d555..7e80fd6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -177,7 +177,7 @@ dependencies = [ [[package]] name = "nix" version = "0.19.0" -source = "git+https://github.com/trailofbits/nix?branch=ww/personality#ab43430c37eb4106d89e2e3832c99419a45fa598" +source = "git+https://github.com/trailofbits/nix?branch=ww/personality#cc47a74139b74fc56325c159783dc36521a14176" dependencies = [ "bitflags", "cc", From 6a21edb0ea13d772e5e70ccd4f32d5b69a1a0745 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 10 Nov 2020 14:58:58 -0500 Subject: [PATCH 3/7] workflows/ci: Use sudo --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index db116e3..2ce85ce 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,7 +27,7 @@ jobs: - uses: actions/checkout@v2 - name: System deps - run: apt install -y nasm + run: sudo apt install -y nasm - name: Build run: cargo build From 308741d5cccdcfca89aa607a0cdea6b04e57012b Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 10 Nov 2020 15:02:06 -0500 Subject: [PATCH 4/7] README: Add a note about unit tests --- README.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/README.md b/README.md index 8f15721..05e47d3 100644 --- a/README.md +++ b/README.md @@ -38,3 +38,14 @@ Once you have the appropriate environment, just `cargo build`: $ cargo build $ ./target/debug/mttn -h ``` + +### Testing + +*mttn*'s tests require some system depedencies to build test binaries with: +`nasm`, (GNU) `ld`, and (GNU) `make`. + +Once you have those installed, running the tests should be as simple as: + +```bash +$ cargo test +``` From f965ccc6933078db378a99788567d369ad4d2d6e Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 10 Nov 2020 15:25:20 -0500 Subject: [PATCH 5/7] trace: Use a macro for consistency tests --- src/trace.rs | 57 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/src/trace.rs b/src/trace.rs index a089a80..bb66015 100644 --- a/src/trace.rs +++ b/src/trace.rs @@ -701,29 +701,40 @@ mod tests { assert!(regs.value(Register::ST0).is_err()); } - #[test] - fn test_trace_consistency() { - for program in &["memops.elf", "stosb.elf", "stosw.elf", "stosd.elf"] { - let program = build_test_program(program); - let tracer = test_program_tracer(&program); - - // TODO(ww): Don't collect these. - let trace1 = tracer - .trace() - .expect("spawn failed") - .collect::>>() - .expect("trace failed"); - - let trace2 = tracer - .trace() - .expect("spawn failed") - .collect::>>() - .expect("trace failed"); - - assert_eq!(trace1, trace2); - for (step1, step2) in trace1.iter().zip(trace2.iter()) { - assert_eq!(step1, step2); - } + macro_rules! trace_consistency_tests { + ($($name:ident,)*) => { + $( + #[test] + fn $name() { + let program = build_test_program(concat!(stringify!($name), ".elf")); + let tracer = test_program_tracer(&program); + + // TODO(ww): Don't collect these. + let trace1 = tracer + .trace() + .expect("spawn failed") + .collect::>>() + .expect("trace failed"); + + let trace2 = tracer + .trace() + .expect("spawn failed") + .collect::>>() + .expect("trace failed"); + + assert_eq!(trace1.len(), trace2.len()); + for (step1, step2) in trace1.iter().zip(trace2.iter()) { + assert_eq!(step1, step2); + } + } + )* } } + + trace_consistency_tests! { + memops, + stosb, + stosw, + stosd, + } } From 0643a3f02fbe53d4db47d159741f79b91988b885 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 10 Nov 2020 15:37:49 -0500 Subject: [PATCH 6/7] trace: Sleep a bit more Not a good fix, but just to get the tests passing. --- src/trace.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/trace.rs b/src/trace.rs index bb66015..08750c2 100644 --- a/src/trace.rs +++ b/src/trace.rs @@ -529,7 +529,7 @@ impl<'a> Tracee<'a> { // fast-string-enable bit (0b) in the IA32_MISC_ENABLE MSR, but we just sleep // for a bit to give the CPU a chance to catch up. // TODO(ww): Longer term, we should just model REP'd instructions outright. - std::thread::sleep(std::time::Duration::from_millis(1)); + std::thread::sleep(std::time::Duration::from_millis(2)); for hint in hints.iter_mut() { if hint.operation != MemoryOp::Write { From bededf10f0221235831a31d5c300b45b7265f136 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Mon, 7 Dec 2020 13:52:45 -0500 Subject: [PATCH 7/7] chore(cargo): cargo update --- Cargo.lock | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bde6555..f681711 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -43,9 +43,9 @@ checksum = "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693" [[package]] name = "cc" -version = "1.0.62" +version = "1.0.66" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f1770ced377336a88a67c473594ccc14eca6f4559217c34f64aac8f83d641b40" +checksum = "4c0496836a84f8d0495758516b8621a622beb77c0fed418570e50764093ced48" [[package]] name = "cfg-if" @@ -156,7 +156,7 @@ dependencies = [ "iced-x86", "libc", "log", - "nix 0.19.1", + "nix 0.19.0", "serde", "serde_json", "spawn-ptrace", @@ -177,7 +177,7 @@ dependencies = [ [[package]] name = "nix" version = "0.19.0" -source = "git+https://github.com/trailofbits/nix?branch=ww/personality#cc47a74139b74fc56325c159783dc36521a14176" +source = "git+https://github.com/trailofbits/nix?branch=ww/personality#328a23093edb90a96eb5a12a8010c92949a5189d" dependencies = [ "bitflags", "cc", @@ -305,9 +305,9 @@ checksum = "8ea5119cdb4c55b55d432abb513a0429384878c15dde60cc77b1c99de1a95a6a" [[package]] name = "syn" -version = "1.0.48" +version = "1.0.53" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cc371affeffc477f42a221a1e4297aedcea33d47d19b61455588bd9d8f6b19ac" +checksum = "8833e20724c24de12bbaba5ad230ea61c3eafb05b881c7c9d3cfe8638b187e68" dependencies = [ "proc-macro2", "quote", @@ -316,9 +316,9 @@ dependencies = [ [[package]] name = "termcolor" -version = "1.1.0" +version = "1.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bb6bfa289a4d7c5766392812c0a1f4c1ba45afa1ad47803c11e1f407d846d75f" +checksum = "2dfed899f0eb03f32ee8c6a0aabdb8a7949659e3466561fc0adf54e26d88c5f4" dependencies = [ "winapi-util", ]