There maybe some bugs in rex prefix judge of 64-bits

[Githubwyb]

when I use subhook to hook this function, the function 'subhook_disasm' return the len of line which in red box to be 11-byte, but actually it is 7-byte. I check the intel document to find out why, but I can't understand it. The picture is I compile a C++ code with

• g++: 4.8.5 20150623
• 64-bits red-hat machine
• kernal info "3.10.0-327.13.1.el7.x86_64"

And I haven't find a machine code with prefix 0x48 has length larger than 7.

Would you mind to check this question? thanks a lot.

[Githubwyb]

The whole machine code here, top 7 line is right, but the line 41c435 return 'len = 11', it copy some wrong byte to trampoline which make it can't run the original function by 'subhook_get_trampoline(hook)'

000000000041c428 <_ZN6Worker15_CheckTcpIPDataEPciPKcP3CPv>:
  41c428:   55                      push   %rbp
  41c429:   48 89 e5                mov    %rsp,%rbp
  41c42c:   41 57                   push   %r15
  41c42e:   41 56                   push   %r14
  41c430:   41 55                   push   %r13
  41c432:   41 54                   push   %r12
  41c434:   53                      push   %rbx
  41c435:   48 81 ec d8 08 00 00    sub    $0x8d8,%rsp
  41c43c:   48 89 bd b8 f7 ff ff    mov    %rdi,-0x848(%rbp)
  41c443:   48 89 b5 b0 f7 ff ff    mov    %rsi,-0x850(%rbp)
  41c44a:   89 95 ac f7 ff ff       mov    %edx,-0x854(%rbp)
  41c450:   48 89 8d a0 f7 ff ff    mov    %rcx,-0x860(%rbp)
  41c457:   4c 89 85 98 f7 ff ff    mov    %r8,-0x868(%rbp)
  41c45e:   48 8b 85 b0 f7 ff ff    mov    -0x850(%rbp),%rax
  41c465:   48 89 45 c8             mov    %rax,-0x38(%rbp)

[shizhx]

Instead of re-inventing a naive disassembler, using Hacker Disassembler Engine(hde32c/hde64c) would be better. :)

[theasmgirl]

The reason of this happening goes from

#ifdef SUBHOOK_X86_64
  if ((code[len] & 0xF0) == 0x40) {
    /* This is a REX prefix (40H - 4FH). REX prefixes are valid only in
     * 64-bit mode.
     */
    uint8_t rex = code[len++];

    if (rex & 8) {
      /* REX.W changes size of immediate operand to 64 bits. */
      operand_size = 8;
    }
  }
#endif

As described in the title of the issue. As you can see, it changes size of the immediate operand to 64 bits, which basically gives us instruction SUB r/64, imm64. And in this case, the subhook's disassembler is correct and length of the instruction is 11 bytes as well. Except, the real instruction looks like

Even with REX.W, the immediate operand size is still 32 bits, but subhook's disassembler doesn't think so. Actually, seems like it would happen with any instruction that provides REX.W but doesn't change immediate operand size. And thinking about it, probably this is also the reason of trampoline failures in x86_64 applications in the previous issues.

On the other hand, let's test it with an instruction which actually provides 64 bit immediate. As an example, we can take mov r64, imm64. Byte code of the example where I provided random 64-bit value to RAX: 0x48, 0xB8, 0xFF, 0xFF, 0xAA, 0xFF, 0xAA, 0xFF, 0x00, 0x00 (10 bytes). The subhook's disassembler gives us: And as you can see it's the correct length.

Talking about the fix, one of the ways probably would be adding IMM64 to the flags, update opcode table and check it with the RAX.W as well. Illustrative example:

Update the flags.

  enum flags {
    MODRM      = 1,
    PLUS_R     = 1 << 1,
    REG_OPCODE = 1 << 2,
    IMM8       = 1 << 3,
    IMM16      = 1 << 4,
    IMM32      = 1 << 5,
    IMM64      = 1 << 6,
    RELOC      = 1 << 7
  };

Add additional flag for the MOV instruction.

/* MOV r32, imm32    */ {0xB8, 0, PLUS_R | IMM32 | IMM64},

Check with the REX prefix.

#ifdef SUBHOOK_X86_64
    /* Check if the REX prefix provided */
    bool rex_provided = false; /* Since we need to check opcode flags later */

    uint8_t rex = code[len];
    uint8_t rex_w;
    if(((rex >> 4) & 0xF) == 0x04)
    {
        rex_provided = true;
        /* Get the REX.W prefix bit */
        rex_w = ((rex >> 3) & 1);

        len++; /* REX prefix */
    }  
#endif

    /* Code of the opcode searching (skipped) */

#ifdef SUBHOOK_X86_64
    if(rex_provided)
    {
        /* After we found the opcode check flags and then change the operand size in case. */
        if((opcodes[i].flags & IMM64) && (rex_w & 1))
        {
            /* Change the immediate operand size to 64 bits */
            operand_size = 8;
        }
    }
#endif

After that, I tested both your case and mine (64-bit mov immediate). Your case: Mine:

As you can see, both cases work fine.

This is just an example of what a fix for this issue might look like. It's up to the author of subhook himself. Also, AFAIK, there are instructions which provide immediate 64 bit value even without REX prefix, which might be a problem on the unix-like systems, but I didn't test it. (though this is also can be fixed with flags)

[theasmgirl]

Also, in addition to my previous comment, it seems like... subhook's disasm doesn't check opcode for expansion prefix? Or I might be wrong? In that case it totally ruins compatibility with 2-byte opcodes though.