Vector35 / binaryninja-api

Public API, examples, documentation and issues for Binary Ninja
https://binary.ninja/
MIT License
926 stars 209 forks source link

BN uses offset instead of structure member at the decompiler output #4166

Open op2786 opened 1 year ago

op2786 commented 1 year ago

Version and Platform (required):

Bug Description: Looks like BN sometimes can not use structure members and uses offsets instead of them.

Example 1:

HLIL:

163d9bf6  thid_t rdi = tid
163d9bfe  if (res_mode - 1 u<= 1) {
163d9c19    if (dbg->engine.engine_status == as_dump || (dbg->engine:0x458.d != as_dump && data_163fb058 == 2)) {
163d9ce9      ida_deb("No runnable debuggees!\n")
163d9ce2    }
163d9c19    if (dbg->engine:0x458.d != as_dump && data_163fb058 != 2) {

Disassembly:

163d9be0  48895c2408         mov     qword [rsp+0x8 {__saved_rbx}], rbx
163d9be5  4889742410         mov     qword [rsp+0x10 {__saved_rsi}], rsi
163d9bea  57                 push    rdi {__saved_rdi}
163d9beb  4883ec20           sub     rsp, 0x20
163d9bef  418d40ff           lea     eax, [r8-0x1]
163d9bf3  418bf0             mov     esi, r8d
163d9bf6  8bfa               mov     edi, edx
163d9bf8  488bd9             mov     rbx, rcx
163d9bfb  83f801             cmp     eax, 0x1
163d9bfe  0f87ea000000       ja      0x163d9cee

163d9c04  83b98007000003     cmp     dword [rcx+0x780], 0x3
163d9c0b  0f84d1000000       je      0x163d9ce2

163d9c11  66833d3f14020002   cmp     word [rel data_163fb058], 0x2
163d9c19  0f84c3000000       je      0x163d9ce2  {data_163fb058}

Offset 0x458 in engine structure is engine_status which is already used in 163d9c19. Actually this is more like else block of the first if block but BN does not generetes a else block and creates a new if block instead of it.

Example 2:

HLIL:

00410ad8  struct proxy_socket_ctx sockets
00410ad8  sockets.remote_socket = ctx
00410ad9  sockets.local_socket = ctx
00410ae6  char* ecx = EnterCriticalSection(&ctx->lock)
00410af3  char* domain_temp
00410af3  int32_t eax_3
00410af3  int32_t result
00410af3  if (ctx->remote_wsa.connected == 0) {
00410af8    int32_t remote_port = thread_ctx->remote_port
00410afe    domain_temp = ecx
00410b02    duplicate_str(&domain_temp, &thread_ctx->remote_addr)
00410b0d    eax_3, ecx = connect_to_cc(&ctx->remote_wsa, domain_temp, remote_port)
00410b14    if (eax_3 == 0) {
00410b38      label_410b38:
00410b38      LeaveCriticalSection(&ctx->lock)
00410b40      result = 1
00410b40    }
00410aec  }
00410b14  if (ctx->remote_wsa:8.d != 0 || (ctx->remote_wsa:8.d == 0 && eax_3 != 0)) {
00410b1a    if (ctx->local_wsa.connected == 0) {
00410b1f      int32_t local_port = thread_ctx->local_port
00410b22      domain_temp = ecx
00410b26      duplicate_str(&domain_temp, thread_ctx)
00410b35      if (connect_to_cc(&ctx->local_wsa, domain_temp, local_port) == 0) {
00410b35        goto label_410b38
00410b35      }
00410b35    }

Disassembly:

00410ad5  55                 push    ebp {__saved_ebp}
00410ad6  8bec               mov     ebp, esp {__saved_ebp}
00410ad8  51                 push    ecx {sockets.remote_socket}
00410ad9  51                 push    ecx {sockets.local_socket}
00410ada  53                 push    ebx {__saved_ebx}  {0x0}
00410adb  56                 push    esi {__saved_esi}
00410adc  57                 push    edi {__saved_edi}
00410add  8bf9               mov     edi, ecx
00410adf  8d9fd8030000       lea     ebx, [edi+0x3d8] {proxy_ctx::lock}
00410ae5  53                 push    ebx {var_1c}
00410ae6  ff158c914100       call    dword [EnterCriticalSection]
00410aec  83bfec01000000     cmp     dword [edi+0x1ec {proxy_ctx::remote_wsa.connected}], 0x0
00410af3  7521               jne     0x410b16

00410af5  8b4508             mov     eax, dword [ebp+0x8 {thread_ctx}]
00410af8  ff700c             push    dword [eax+0xc {proxy_thread_ctx::remote_port}] {remote_port}
00410afb  83c008             add     eax, 0x8 {proxy_thread_ctx::remote_addr}
00410afe  51                 push    ecx {domain_temp}
00410aff  8bcc               mov     ecx, esp {domain_temp}
00410b01  50                 push    eax {var_24_1}
00410b02  e87e38ffff         call    duplicate_str
00410b07  8d8fe4010000       lea     ecx, [edi+0x1e4] {proxy_ctx::remote_wsa}
00410b0d  e85f5dffff         call    connect_to_cc
00410b12  85c0               test    eax, eax
00410b14  7421               je      0x410b37

00410b16  837f0c00           cmp     dword [edi+0xc {proxy_ctx::local_wsa.connected}], 0x0
00410b1a  752a               jne     0x410b46

00410b1c  8b4508             mov     eax, dword [ebp+0x8 {thread_ctx}]
00410b1f  ff7004             push    dword [eax+0x4 {proxy_thread_ctx::local_port}] {local_port}
00410b22  51                 push    ecx {domain_temp}
00410b23  8bcc               mov     ecx, esp {domain_temp}
00410b25  50                 push    eax {var_24_2}
00410b26  e85a38ffff         call    duplicate_str
00410b2b  8d4f04             lea     ecx, [edi+0x4] {proxy_ctx::local_wsa}
00410b2e  e83e5dffff         call    connect_to_cc
00410b33  85c0               test    eax, eax
00410b35  750f               jne     0x410b46

remote_wsa:8 is connected in the wsa_ctx structure, which already used at 00410af3.

Steps To Reproduce: Actually, I don't know how to reproduce these. But I can share BNDB and binaries in slack.

Expected Behavior: I expect it to use structure member.

ccarpenter04 commented 1 year ago

It seems like there are a few issues being shown here, perhaps they should be split up into individual issues?