From 81e9b93aab9217cf7cb508f64b19fc1c0df024b5 Mon Sep 17 00:00:00 2001 From: iximeow Date: Thu, 19 Nov 2020 18:13:24 -0800 Subject: fix decoding of rex-prefixed modrm+sib operands selecting index 0b100 and base 0b101 for memory operands with a base, index, and displacement either the wrong base would be selected (register number ignored, so only `*ax` or `r8*` would be reported), or yaxpeax-x86 would report a base register is present when it is not (`RegIndexBaseScaleDisp` when the operand is actually `RegScaleDisp`) thank you to Evan Johnson for catching and reporting this bug! also bump crate version to 0.1.4 as this will be immediately tagged and released. --- CHANGELOG | 10 ++++++++++ Cargo.toml | 2 +- src/long_mode/mod.rs | 16 ++++++++++++---- test/long_mode/mod.rs | 11 +++++++++++ test/protected_mode/mod.rs | 34 ++++++++++++++++++++++++++++++++++ 5 files changed, 68 insertions(+), 5 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 5f0b9f7..8de4801 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,13 @@ +## 0.1.4 +* [long mode only]: fix decoding of rex-prefixed modrm+sib operands selecting index 0b100 and base 0b101 + - for memory operands with a base, index, and displacement either + the wrong base would be selected (register number ignored, so only + `*ax` or `r8*` would be reported), or yaxpeax-x86 would report a + base register is present when it is not (`RegIndexBaseScaleDisp` + when the operand is actually `RegScaleDisp`) + +thank you to Evan Johnson for catching and reporting this bug! + ## 0.1.3 * fix 0x80-opcode instructions not having an opcode - this meant that for example `lock xorb [rax], 0` would decode as invalid diff --git a/Cargo.toml b/Cargo.toml index c6da196..dda7c14 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "yaxpeax-x86" -version = "0.1.3" +version = "0.1.4" authors = [ "iximeow " ] license = "0BSD" repository = "http://git.iximeow.net/yaxpeax-x86/" diff --git a/src/long_mode/mod.rs b/src/long_mode/mod.rs index 363c605..d165e38 100644 --- a/src/long_mode/mod.rs +++ b/src/long_mode/mod.rs @@ -5665,20 +5665,28 @@ fn read_sib>(bytes_iter: &mut T, instr: &mut Instruction, m OperandSpec::DispU32 } else { + instr.modrm_mmm.num |= 0b101; + if instr.prefixes.rex().x() { instr.sib_index.num = 0b1100; let scale = 1u8 << (sibbyte >> 6); instr.scale = scale; if disp == 0 { - OperandSpec::RegIndexBaseScale + if modbits == 0 { + OperandSpec::RegScale + } else { + OperandSpec::RegIndexBaseScale + } } else { instr.disp = disp as i64 as u64; - OperandSpec::RegIndexBaseScaleDisp + if modbits == 0 { + OperandSpec::RegScaleDisp + } else { + OperandSpec::RegIndexBaseScaleDisp + } } } else { - instr.modrm_mmm.num |= 0b101; - if disp == 0 { OperandSpec::Deref } else { diff --git a/test/long_mode/mod.rs b/test/long_mode/mod.rs index 3b31dbb..8489822 100644 --- a/test/long_mode/mod.rs +++ b/test/long_mode/mod.rs @@ -109,6 +109,17 @@ fn test_modrm_decode() { test_display(&[0x41, 0x33, 0x84, 0xa5, 0x11, 0x22, 0x33, 0x44], "xor eax, [r13 + 0x44332211]"); test_display(&[0x33, 0x04, 0xe5, 0x11, 0x22, 0x33, 0x44], "xor eax, [0x44332211]"); test_display(&[0x41, 0x33, 0x04, 0xe5, 0x11, 0x22, 0x33, 0x44], "xor eax, [0x44332211]"); + + // specifically sib with base == 0b101 + // mod bits 00 + test_display(&[0x42, 0x33, 0x34, 0x25, 0x20, 0x30, 0x40, 0x50], "xor esi, [r12 * 1 + 0x50403020]"); + test_display(&[0x43, 0x33, 0x34, 0x25, 0x20, 0x30, 0x40, 0x50], "xor esi, [r12 * 1 + 0x50403020]"); + // mod bits 01 + test_display(&[0x42, 0x33, 0x74, 0x25, 0x20], "xor esi, [rbp + r12 * 1 + 0x20]"); + test_display(&[0x43, 0x33, 0x74, 0x25, 0x20], "xor esi, [r13 + r12 * 1 + 0x20]"); + // mod bits 10 + test_display(&[0x42, 0x33, 0xb4, 0x25, 0x20, 0x30, 0x40, 0x50], "xor esi, [rbp + r12 * 1 + 0x50403020]"); + test_display(&[0x43, 0x33, 0xb4, 0x25, 0x20, 0x30, 0x40, 0x50], "xor esi, [r13 + r12 * 1 + 0x50403020]"); } #[test] diff --git a/test/protected_mode/mod.rs b/test/protected_mode/mod.rs index 84448ef..dd0a51f 100644 --- a/test/protected_mode/mod.rs +++ b/test/protected_mode/mod.rs @@ -50,6 +50,40 @@ fn test_display_under(decoder: &InstDecoder, data: &[u8], expected: &'static str } #[test] +fn test_modrm_decode() { + // just modrm + test_display(&[0x33, 0x08], "xor ecx, [eax]"); + test_display(&[0x33, 0x20], "xor esp, [eax]"); + test_display(&[0x33, 0x05, 0x78, 0x56, 0x34, 0x12], "xor eax, [0x12345678]"); + test_display(&[0x33, 0x41, 0x23], "xor eax, [ecx + 0x23]"); + test_display(&[0x33, 0x81, 0x23, 0x01, 0x65, 0x43], "xor eax, [ecx + 0x43650123]"); + test_display(&[0x33, 0xc1], "xor eax, ecx"); + + // sib + test_display(&[0x33, 0x04, 0x0a], "xor eax, [edx + ecx * 1]"); + test_display(&[0x33, 0x04, 0x4a], "xor eax, [edx + ecx * 2]"); + test_display(&[0x33, 0x04, 0x8a], "xor eax, [edx + ecx * 4]"); + test_display(&[0x33, 0x04, 0xca], "xor eax, [edx + ecx * 8]"); + test_display(&[0x33, 0x04, 0x20], "xor eax, [eax]"); + test_display(&[0x33, 0x04, 0x60], "xor eax, [eax]"); + test_display(&[0x33, 0x04, 0xa0], "xor eax, [eax]"); + test_display(&[0x33, 0x04, 0xe0], "xor eax, [eax]"); + test_display(&[0x33, 0x04, 0x25, 0x11, 0x22, 0x33, 0x44], "xor eax, [0x44332211]"); + + test_display(&[0x33, 0x44, 0x65, 0x11], "xor eax, [ebp + 0x11]"); + test_display(&[0x33, 0x84, 0xa5, 0x11, 0x22, 0x33, 0x44], "xor eax, [ebp + 0x44332211]"); + test_display(&[0x33, 0x04, 0xe5, 0x11, 0x22, 0x33, 0x44], "xor eax, [0x44332211]"); + + // specifically sib with base == 0b101 + // mod bits 00 + test_display(&[0x33, 0x34, 0x25, 0x20, 0x30, 0x40, 0x50], "xor esi, [0x50403020]"); + // mod bits 01 + test_display(&[0x33, 0x74, 0x25, 0x20], "xor esi, [ebp + 0x20]"); + // mod bits 10 + test_display(&[0x33, 0xb4, 0x25, 0x20, 0x30, 0x40, 0x50], "xor esi, [ebp + 0x50403020]"); +} + +#[test] fn test_mmx() { test_display(&[0x0f, 0xf7, 0xc1], "maskmovq mm0, mm1"); test_invalid(&[0x0f, 0xf7, 0x01]); -- cgit v1.1