search

Vector35 / binaryninja-api

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

Structure fields not recognized in an array of structures #5376

Open joelreymont opened 5 months ago

joelreymont commented 5 months ago

Version and Platform (required):

Internal binary major dine favor.

I'm getting pretty bad output from BN and it's not letting me tweak it. For example, that 0x145e4c is the second field in the structure. And I don't understand why it's trying to grab series1 and x1_5 as 64-bit ints instead of splitting them into separate structure field accesses. 

00099c68          if (PAYLOAD_NEGOTIATE_PARAM_CONFIG_LIST[sx.q(ix)].series == series && *(sx.q(ix) * 0x14 + 0x145e4c) == mount_pos_type)
00099cac              int64_t ix_1 = sx.q(ix)
00099ccc              int64_t series_1 = PAYLOAD_NEGOTIATE_PARAM_CONFIG_LIST[ix_1].series.q
00099ccc              int64_t x1_5 = PAYLOAD_NEGOTIATE_PARAM_CONFIG_LIST[ix_1].no_negotiate_flag.q
00099cd0              config->series = series_1.d
00099cd0              config->mount_pos_type = series_1:4.d
00099cd0              config->no_negotiate_flag = x1_5.d
00099cd0              config->sender = x1_5:4.b
00099cd0              config->receiver = x1_5:5.b
00099cd0              config->field5 = x1_5:6.w
00099cd8              config->field6.d = PAYLOAD_NEGOTIATE_PARAM_CONFIG_LIST[ix_1].field_6
00099cdc              x0_1 = 0
00099ce0              break

Both of series1 and x1_5 are picking up data from the same structure and should be automatically split into several field accesses instead. PAYLOAD_NEGOTIATE_PARAM_CONFIG_LIST is an array of structures that looks like this

00145e48  struct PayloadNegotiateParamConfig PAYLOAD_NEGOTIATE_PARAM_CONFIG_LIST[0x9] = 
00145e48  {
00145e48      [0x0] = 
00145e48      {
00145e48          enum E_DjiAircraftSeries series = DJI_AIRCRAFT_SERIES_M300
00145e4c          enum E_DjiMountPositionType mount_pos_type = DJI_MOUNT_POSITION_TYPE_PAYLOAD_PORT
00145e50          uint32_t no_negotiate_flag = 0x0
00145e54          uint8_t sender = 0x13
00145e55          uint8_t receiver = 0x34
00145e56          uint16_t field_5 = 0x9
00145e58          uint32_t field_6 = 0x2710
00145e5c      },
...

Let me know if I should upload my database somewhere.

joelreymont commented 5 months ago

I can trick BN into doing the right thing using a couple of anonymous structures. I think BN should be able to figure it out by itself, though.

00099c68          if (PAYLOAD_NEGOTIATE_PARAM_CONFIG_LIST[sx.q(ix)].series == series && *(sx.q(ix) * 0x14 + 0x145e4c) == mount_pos_type)
00099cac              int64_t ix_1 = sx.q(ix)
00099ccc              struct anonymous_0
00099ccc              anonymous_0.series = PAYLOAD_NEGOTIATE_PARAM_CONFIG_LIST[ix_1].series
00099ccc              anonymous_0.mount_pos_type = PAYLOAD_NEGOTIATE_PARAM_CONFIG_LIST[ix_1].mount_pos_type
00099ccc              struct anonymous_0_1
00099ccc              anonymous_0_1.no_negotiate_flag = PAYLOAD_NEGOTIATE_PARAM_CONFIG_LIST[ix_1].no_negotiate_flag
00099ccc              anonymous_0_1.sender = PAYLOAD_NEGOTIATE_PARAM_CONFIG_LIST[ix_1].sender
00099ccc              anonymous_0_1.receiver = PAYLOAD_NEGOTIATE_PARAM_CONFIG_LIST[ix_1].receiver
00099ccc              anonymous_0_1.foo = PAYLOAD_NEGOTIATE_PARAM_CONFIG_LIST[ix_1].field_5
00099cd0              config->series = anonymous_0.series
00099cd0              config->mount_pos_type = anonymous_0.mount_pos_type
00099cd0              config->no_negotiate_flag = anonymous_0_1.no_negotiate_flag
00099cd0              config->sender = anonymous_0_1.sender
00099cd0              config->receiver = anonymous_0_1.receiver
00099cd0              config->field5 = anonymous_0_1.foo
00099cd8              config->field6.d = PAYLOAD_NEGOTIATE_PARAM_CONFIG_LIST[ix_1].field_6
00099cdc              err = 0
00099ce0              break
00099cec          ix = ix + 1

Also, there's still the issue of *(sx.q(ix) * 0x14 + 0x145e4c) == mount_pos_type not recognized as PAYLOAD_NEGOTIATE_PARAM_CONFIG_LIST[sx.q(ix)].mount_pos_type == mount_pos_type.

joelreymont commented 5 months ago

Here a much better example where the problem is much worse...

struct CmdWaitItem __packed
{
    uint32_t is_used;
    uint32_t timeout;
    uint32_t size;
    uint32_t flag;
    struct PacketMetaData meta;
    void* payload;
    void* callback;
    struct SyncInfo* sync_info;
};

struct Command __packed
{
    void* mutex;
    __padding char _8[0x48];
    __padding char _50[0x50];
    __padding char _a0[0x50];
    __padding char _f0[0x50];
    __padding char _140[0x50];
    __padding char _190[0x50];
    __padding char _1e0[0x28];
    void* mutex_1;
    struct CmdWaitItem wait_items[0x20];
    struct ChildTaskNode task;
    int32_t field_a40;
    __padding char _a44[4];
    struct ListNode linker_list;
    int16_t node_count;
    __padding char _a5a[6];
    struct Linker linkers[0x5];
    void* mutex_2;
    int16_t seqnum;
    char field_d8a;
    __padding char _d8b[5];
    void* field_d90;
    void* mem_3ff;
};

Note the super-weird structure field references into items of wait_items, an array of struct CmdWaitItem. Also note that there are straightforward field accesses into the same structure, e.g. cmd->wait_items[sx.q(ix)].is_used and cmd->wait_items[sx.q(ix)].timeout.

00063390                      if (zx.d(cmd->wait_items[sx.q(ix)].is_used.b ^ 1) == 0)
000635cc                          ix = ix + 1
000635cc                          continue
00063390                      else
000633c4                          if (CMD_OSAL->GetTimeMs(ms: &cmd->wait_items[sx.q(ix)].timeout) != 0)
000633f0                              DjiLogger_UserLogOutput(level: "linker", fmt: nullptr, "[%s:%d) get system time error", "DjiCommand_SendAsync", 0x1cb)
000633f8                              err = 0xec
0006340c                          (&cmd->wait_items[0]:8.d)[sx.q(ix) * 0x10] = size
00063428                          (&cmd->wait_items[0]:0xc.d)[sx.q(ix) * 0x10].w = flag
00063450                          memcpy(&cmd->wait_items[0]:0x10.24 + (sx.q(ix) << 6), meta, 0x18)
00063470                          if (meta->payload_size != 0 && meta->payload_size u<= 0x3ff)
000634a4                              (&cmd->wait_items[0]:0x28.q)[sx.q(ix) * 8] = CMD_OSAL->Malloc(size: meta->payload_size)
000634c8                              if ((&cmd->wait_items[0]:0x28.q)[sx.q(ix) * 8] == 0)
000634f4                                  DjiLogger_UserLogOutput(level: "linker", fmt: nullptr, "[%s:%d) malloc wait item data er…", "DjiCommand_SendAsync", 0x1d4)
000634f8                                  err_1 = 0xe2
000634fc                                  break
00063530                              memset((&cmd->wait_items[0]:0x28.q)[sx.q(ix) * 8], 0, zx.q(meta->payload_size))
00063564                              memcpy((&cmd->wait_items[0]:0x28.q)[sx.q(ix) * 8], payload, zx.q(meta->payload_size))
00063580                          *(cmd + ((sx.q(ix) + 9) << 6)) = callback
000635a0                          *(cmd + ((sx.q(ix) + 9) << 6) + 8) = sync_info
000635b0                          cmd->wait_items[sx.q(ix)].is_used.b = 1
000635e4                  if (ix == 0x20)
00063610                      DjiLogger_UserLogOutput(level: "linker", fmt: nullptr, "[%s:%d) not have enough resource…", "DjiCommand_SendAsync", 0x1e4)
00063618                      err = 0x101
000635e4                  else
00063674                      for (int32_t i = 0; i s<= 0x1f; i = i + 1)
00063650                          if (zx.d(cmd->wait_items[sx.q(ix)].is_used.b) != 0 && ix s<= 0x1f)
0006365c                              max = max + 1
joelreymont commented 4 months ago

Another example...

Positioning on the first 0x131960 below 

00066fc8          if (*(0x131960 + sx.q(ix) * 0x14) == series && *(sx.q(ix) * 0x14 + 0x131964) == mount_pos_type)

and typing 'O' converts the code to 

00066fc8          if (*(&s_coreParamConfigList + sx.q(ix) * 0x14) == series && *(sx.q(ix) * 0x14 + 0x131964) == mount_pos_type)

BN recognized the variable at the first instance of 0x131960 but not the second because it's offset. Typing

The full function 

00066f48  int64_t DjiCoreParamConfig_Get(int64_t* arg1)

00066f58      enum E_DjiAircraftSeries series
00066f58      DjiAccessAdapter_GetAircraftSeries(series: &series)
00066f78      enum E_DjiAircraftSeries mount_pos_type
00066f78      DjiAccessAdapter_GetMountPositionType(mount_pos_type: &mount_pos_type)
00066f94      int32_t ix = 0
00067058      int64_t err
00067058      while (true)
00067058          if (ix u> 0xa)
00067094              DjiLogger_Output(tag: "utils", level: 0, fmt: "[%s:%d) Can't find module param …", "DjiCoreParamConfig_Get", 0xab, zx.q(series), zx.q(mount_pos_type))
00067098              err = 0x100
00067098              break
00066fc8          if (*(0x131960 + sx.q(ix) * 0x14) == series && *(sx.q(ix) * 0x14 + 0x131964) == mount_pos_type)
0006700c              int64_t ix_1 = sx.q(ix)
0006702c              int64_t x1_5 = *(ix_1 * 0x14 + 0x131968)
00067030              *arg1 = *(0x131960 + ix_1 * 0x14)
00067030              arg1[1] = x1_5
00067038              arg1[2].d = *(ix_1 * 0x14 + 0x131970)
0006703c              err = 0
00067040              break
0006704c          ix += 1
000670a0      return err

I went ahead and created a structure type 

struct CoreParamConfig __packed
{
    __padding char _0[0x14];
};

then set the type of s_coreParamConfigList to struct CoreParamConfig s_coreParamConfigList[0xa].

The code changed some 

00066fc8          if (s_coreParamConfigList[sx.q(ix)].__offset(0x0).d == series && *(sx.q(ix) * 0x14 + 0x131964) == mount_pos_type)
0006700c              int64_t ix_1 = sx.q(ix)
0006702c              int64_t x1_5 = *(ix_1 * 0x14 + 0x131968)
00067030              *arg1 = *(0x131960 + ix_1 * 0x14)
00067030              arg1[1] = x1_5
00067038              arg1[2].d = *(ix_1 * 0x14 + 0x131970)
0006703c              err = 0
00067040              break

but pressing 'O' on that 0x131964 still didn't do a thing.

I pointed at .__offset(0x0).d and created a field for that member.

My structure now looks like this 

struct CoreParamConfig __packed
{
    int32_t field_0;
};

and the code looks like this

00066fc8          if (s_coreParamConfigList[sx.q(ix)].field_0 == series && *(sx.q(ix) * 0x14 + 0x131964) == mount_pos_type)
0006700c              int64_t ix_1 = sx.q(ix)
0006702c              int64_t x1_5 = *(ix_1 * 0x14 + 0x131968)
00067030              *arg1 = *(0x131960 + ix_1 * 0x14)
00067030              arg1[1] = x1_5
00067038              arg1[2].d = *(ix_1 * 0x14 + 0x131970)
0006703c              err = 0
00067040              break

'O' on 0x131964 doesn't do anything. 'O' on 0x131970 doesn't do anything.

'O' on 0x131960 changes the code to 

00066fc8          if (s_coreParamConfigList[sx.q(ix)].field_0 == series && *(sx.q(ix) * 0x14 + 0x131964) == mount_pos_type)
0006700c              int64_t ix_1 = sx.q(ix)
0006702c              int64_t x1_5 = s_coreParamConfigList[ix_1].__offset(0x8)
00067030              *arg1 = s_coreParamConfigList[ix_1].field_0.q
00067030              arg1[1] = x1_5
00067038              arg1[2].d = s_coreParamConfigList[ix_1].__offset(0x10)
0006703c              err = 0
00067040              break

BN lets me use 'S' on the offset to Create current member for structure multiple times giving me this bogus structure type

struct CoreParamConfig __packed
{
    int32_t field_0;
    char field_8[0x0];
    char field_8[0x0];
    char field_10[0x0];
    char field_10[0x0];
};

but reanalyzing function doesn't make it use the fields.

Manually typing the structure as 

struct CoreParamConfig __packed
{
    int32_t field_0;
    uint32_t field_1;
    uint32_t field_2;
    uint32_t field_3;
    uint32_t field_4;
};

does fix the issue after a couple of seconds so I get the fields

00066fc8          if (s_coreParamConfigList[sx.q(ix)].field_0 == series && *(sx.q(ix) * 0x14 + 0x131964) == mount_pos_type)
0006700c              int64_t ix_1 = sx.q(ix)
0006702c              int64_t x1_5 = s_coreParamConfigList[ix_1].field_2.q
00067030              *arg1 = s_coreParamConfigList[ix_1].field_0.q
00067030              arg1[1] = x1_5
00067038              arg1[2].d = s_coreParamConfigList[ix_1].field_4
0006703c              err = 0
00067040              break

but still no array access for *(sx.q(ix) * 0x14 + 0x131964).