radareorg / radare2

UNIX-like reverse engineering framework and command-line toolset
https://www.radare.org/
GNU Lesser General Public License v3.0
20.67k stars 3k forks source link

Invalid jmptbl size on x86-64 #14505

Open radare opened 5 years ago

radare commented 5 years ago

Problem is related to the recursive analysis of some jumptables.

Here's the jmp rax:

$ r2 /bin/ls
[0x1000011e8]> s..1bf4
[0x100001bf4]> pd 1
            0x100001bf4      ffe0           jmp   rax
[0x100001bf4]>

after analysis we get only 2 cases

[0x100001bf4]> aaaa
[x] Analyze all flags starting with sym. and entry0 (aa)
[x] Analyze function calls (aac)
[x] Analyze len bytes of instructions for references (aar)
[x] Check for objc references
[x] Check for vtables
[x] Type matching analysis for all functions (aaft)
[x] Use -AA or aaaa to perform additional experimental analysis.
[x] Finding function preludes
[x] Enable constraint types analysis for variables
[0x100001bf4]> pd 2
│           ;-- switch.0x100001bf4:
│           0x100001bf4      ffe0           jmp   rax                        ; switch table (2 cases) at 0x100001dd4
            0x100001bf6      418b7d38       mov   edi,  dword [r13 + 0x38]   ; [0x38:4]=-1 ; '8' ; 56
[0x100001bf4]>

if we disassemble back and chk the jmptable we see the comparison is 6, not 2

[0x100001bf4]> pd-5
│           0x100001be5      83fa06         cmp   edx,  6                    ; 6
│       ┌─< 0x100001be8      7738           ja    0x100001c22
│       │   0x100001bea      0fb7c1         movzx eax,  cx
│       │   0x100001bed      49630487       movsxd rax,  dword [r15 + rax*4]
│       │   0x100001bf1      4c01f8         add   rax,  r15
[0x100001bf4]>

by chking the jmptbl we can observer this:

[0x100001de9]> pd 10 @ 0x100001dd4
│           ; DATA XREF from sym.func.100001b2c (0x100001bd4)
└           0x100001dd4      .int32 -365
            0x100001dd8      .int32 -308
            0x100001ddc      9d             popfq
            0x100001ddd      ff             invalid
            0x100001dde      ff             invalid
            0x100001ddf      ff22           jmp   qword [rdx]
            0x100001de1      fe             invalid
            0x100001de2      ff             invalid
            0x100001de3      ff9dffffff9d   lcall [rbp - 0x62000001]
            0x100001de9      ff             invalid
[0x100001de9]> pxw 4*10 @ 0x100001dd4
0x100001dd4  0xfffffe93 0xfffffecc 0xffffff9d 0xfffffe22  ............"...
0x100001de4  0xffffff9d 0xffffff9d 0xfffffe22 0xe5894855  ........"...UH..
0x100001df4  0x0f3f8b48 0x31584fb7                        H.?..OX1
[0x100001de9]>

there are actually 7 destinations for this jmp rax. in fact the code that fills de gap with cmpval+1, but seems like because of the order to bb analysis the cmpval is lost when reaching the jmp rax.

binary: ls.zip

radare commented 5 years ago

We need a hint to tell r2 how big the jmptable for a specific JMP RAX is. and ignore the cmpval

radare commented 5 years ago

Implemented ahv to set anal hints on the value of the jmp rax to redefine the jmptbl size

https://github.com/radare/radare2/pull/14508

d4em0n commented 4 years ago

Any reason why radare find jmptbl size in before bb first not in current bb first?

                if (is_delta_pointer_table (anal, fcn, op.addr, op.ptr, &jmptbl_addr, &jmp_aop)) {
                    ut64 table_size, default_case = 0;
                    // we require both checks here since try_get_jmptbl_info uses
                    // BB info of the final jmptbl jump, which is no present with
                    // is_delta_pointer_table just scanning ahead
                    // try_get_delta_jmptbl_info doesn't work at times where the
                    // lea comes after the cmp/default case cjmp, which can be
                    // handled with try_get_jmptbl_info
                    if (try_get_jmptbl_info (anal, fcn, jmp_aop.addr, bb, &table_size, &default_case)
                        || try_get_delta_jmptbl_info (anal, fcn, jmp_aop.addr, op.addr, &table_size, &default_case)) {
                        ret = try_walkthrough_jmptbl (anal, fcn, depth, jmp_aop.addr, jmptbl_addr, op.ptr, 4, table_size, default_case, 4);
                        if (ret) {
                            lea_jmptbl_ip = jmp_aop.addr;
                        }
                    }
                }
                r_anal_op_fini (&jmp_aop);
            }
d4em0n commented 4 years ago

changing the order fix the issue

                if (is_delta_pointer_table (anal, fcn, op.addr, op.ptr, &jmptbl_addr, &jmp_aop)) {
                    ut64 table_size, default_case = 0;
                    // we require both checks here since try_get_jmptbl_info uses
                    // BB info of the final jmptbl jump, which is no present with
                    // is_delta_pointer_table just scanning ahead
                    // try_get_delta_jmptbl_info doesn't work at times where the
                    // lea comes after the cmp/default case cjmp, which can be
                    // handled with try_get_jmptbl_info
                    if (try_get_delta_jmptbl_info (anal, fcn, jmp_aop.addr, op.addr, &table_size, &default_case) 
                      || try_get_jmptbl_info (anal, fcn, jmp_aop.addr, bb, &table_size, &default_case) ) {
                        ret = try_walkthrough_jmptbl (anal, fcn, depth, jmp_aop.addr, jmptbl_addr, op.ptr, 4, table_size, default_case, 4);
                        if (ret) {
                            lea_jmptbl_ip = jmp_aop.addr;
                        }
                    }
                }

Screenshot_20200117_090752

radare commented 4 years ago

Can you make a PR?

On 17 Jan 2020, at 03:08, d4em0n notifications@github.com wrote:

 changing the order fix the issue

          if (is_delta_pointer_table (anal, fcn, op.addr, op.ptr, &jmptbl_addr, &jmp_aop)) {
              ut64 table_size, default_case = 0;
              // we require both checks here since try_get_jmptbl_info uses
              // BB info of the final jmptbl jump, which is no present with
              // is_delta_pointer_table just scanning ahead
              // try_get_delta_jmptbl_info doesn't work at times where the
              // lea comes after the cmp/default case cjmp, which can be
              // handled with try_get_jmptbl_info
              if (try_get_delta_jmptbl_info (anal, fcn, jmp_aop.addr, op.addr, &table_size, &default_case) 
                || try_get_jmptbl_info (anal, fcn, jmp_aop.addr, bb, &table_size, &default_case) ) {
                  ret = try_walkthrough_jmptbl (anal, fcn, depth, jmp_aop.addr, jmptbl_addr, op.ptr, 4, table_size, default_case, 4);
                  if (ret) {
                      lea_jmptbl_ip = jmp_aop.addr;
                  }
              }
          }

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

trufae commented 3 years ago

Can you re-check if the issue is still in r2 from git?