From 110f797005cca70e18cbcc0975397d26d8045245 Mon Sep 17 00:00:00 2001 From: iximeow Date: Sat, 16 Dec 2023 13:26:48 -0800 Subject: fix opportunity for unhandled register synonyms registers `al`, `cl`, `dl`, and `bl` could have two different representations - with `rex.w` and without. these two forms of `RegSpec` would not compare equal, nor has the same, so for code relying on `RegSpec` to faithfully represent a 1-1 mapping to x86 registers, these synonyms would introduce bugs in register analysis. for example, in `yaxpeax-core`, this would result in instructions writing to `rex.w al` not being visible as definitions for a future read of `!rex.w al`. fix this in `x86_64` code, add new test cases about the confusion, adjust register names to make this situation more clearly a bug, and introduce two new fuzz targets that would have helped spot this error. --- test/long_mode/mod.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) (limited to 'test') diff --git a/test/long_mode/mod.rs b/test/long_mode/mod.rs index 1e8a72e..25a303c 100644 --- a/test/long_mode/mod.rs +++ b/test/long_mode/mod.rs @@ -3456,6 +3456,36 @@ fn from_reports() { test_display(&[0xf3, 0x0f, 0x1e, 0x0f], "nop dword [rdi], ecx"); } +/// the first four single byte registers (`al`, `cl`, `dl`, `bl`) can be described as either the +/// first four registers of the old `B` bank (`al..bh`) or the first four registers of the new `rB` +/// bank, used when `rex.w` is present on the instruction. `RegSpec` relies on the bank matching to +/// compare equality, as well as for `Hash` and `Ord` to work correctly, so having two spellings of +/// the register `al` (and friends) can be a correctness issue for dependent code. +#[test] +fn register_synonyms_use_old_bank() { + // a few instructions where adding a rex.w prefix would yield a synonymous byte-size register + let cases: &'static [&'static [u8]] = &[ + &[0x30, 0x00], + &[0x30, 0xc1], + &[0x0f, 0xb6, 0xc0], + &[0x0f, 0xbe, 0xc0], + &[0xb2, 0xff], // from `does_not_decode_invalid_registers` fuzz target 🎉 + ]; + + for case in cases.iter() { + let mut case_with_rexw: Vec = vec![0x48]; + case_with_rexw.extend_from_slice(case); + + let mut reader = yaxpeax_arch::U8Reader::new(case); + let no_rexw = InstDecoder::default().decode(&mut reader).expect("can disassemble test instruction"); + + let mut reader = yaxpeax_arch::U8Reader::new(case); + let with_rexw = InstDecoder::default().decode(&mut reader).expect("can disassemble test instruction"); + + assert_eq!(no_rexw, with_rexw); + } +} + mod reg_specs { use yaxpeax_x86::long_mode::RegSpec; -- cgit v1.1