Vector35 / binaryninja-api

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

Switch statement issue #4393

Closed joelreymont closed 1 year ago

joelreymont commented 1 year ago

Version and Platform (required):

Consider the following function ...

00044554  int32_t sub_44554(void* arg1, int32_t arg2, int32_t* arg3)

00044554  {
00044558      int32_t lr;
00044558      int32_t var_4 = lr;
0004455c      int32_t* r4 = *(uint32_t*)((char*)arg1 + 8);
00044560      int32_t r0_3;
00044560      if ((0xef9dffff + arg2) > 0x1f)
0004455e      {
0004458a      label_4458a:
0004458a          r0_3 = 0xffffffea;
000445be      label_445be:
000445be          return r0_3;
000445be      }
00044562      int32_t r3_1;
00044562      int32_t r3_3;
00044562      int32_t r3_4;
00044562      int32_t r3_5;
00044562      int32_t r3_6;
00044562      int32_t r3_7;
00044562      int32_t r3_8;
00044562      switch (*(uint8_t*)(arg2 + 0xefa24565))
...

IDA Pro is able to figure out the values

int __fastcall sub_44554(int a1, int a2, _DWORD *a3)
{
  int v4; // r4
  int (__fastcall *v5)(int, _DWORD *); // r3
  int result; // r0
  int (__fastcall *v7)(_DWORD, _DWORD *); // r2
  int (*v8)(void); // r3
  int (__fastcall *v9)(_DWORD); // r3
  void (__fastcall *v10)(_DWORD, _DWORD *); // r3
  void (__fastcall *v11)(_DWORD *); // r3
  void (*v12)(void); // r3
  int (__fastcall *v13)(_DWORD *); // r3
  int (*v14)(void); // r3
  int (__fastcall *v15)(_DWORD); // r3

  v4 = *(a1 + 8);
  switch ( a2 )
  {
    case 0x10620001:
      v5 = *v4;
      goto LABEL_3;
    case 0x10620002:
      v7 = *(v4 + 4);
      if ( !v7 )
        goto LABEL_4;
      v8 = *(v4 + 92);
      if ( v8 )
      {
        if ( v8() )
 ...

and create code that's much shorter and much cleaner, e.g. without bits like this

000445d6      if (((*(uint8_t*)(arg2 + 0xefa24565) == 0x36 || *(uint8_t*)(arg2 + 0xefa24565) == 0x44) && r3_3 == 0))
000445d4      {
000445d6          goto label_4458a;
000445d6      }
00044614      if (((((((((*(uint8_t*)(arg2 + 0xefa24565) == 0x55 || *(uint8_t*)(arg2 + 0xefa24565) == 0x5c) || *(uint8_t*)(arg2 + 0xefa24565) == 0x5e) || *(uint8_t*)(arg2 + 0xefa24565) == 0x66) || *(uint8_t*)(arg2 + 0xefa24565) == 0x6f) || *(uint8_t*)(arg2 + 0xefa24565) == 0x76) || *(uint8_t*)(arg2 + 0xefa24565) == 0x89) || *(uint8_t*)(arg2 + 0xefa24565) == 0x8d) || *(uint8_t*)(arg2 + 0xefa24565) == 0x8f))
00044562      {
00044614          if (r3_7 != 0)
00044612          {
❓0004461c              /* jump -> r3_7 */
0004461c          }
00044614          goto label_4458a;
00044614      }
00044588      if ((((*(uint8_t*)(arg2 + 0xefa24565) == 0x10 || *(uint8_t*)(arg2 + 0xefa24565) == 0x34) && r3_1 != 0) || *(uint8_t*)(arg2 + 0xefa24565) == 0x15))
00044562      {
❓000445b4          /* jump -> r3_1 */
000445b4      }

Anyone from V35 should search for "Encourage Salesman Prompt Delay" to find the database.

plafosse commented 1 year ago

We could for sure do a better job on this switch statement. We do however do a better job than IDA on the first part of the switch statement as we're recovering proper case labels: image

plafosse commented 1 year ago

This actually seems to be the same issue as this: https://github.com/Vector35/binaryninja-api/issues/1723