Vector35 / Vector35/binaryninja-api

Suboptimal HLIL goto label placement

Open
#1,705 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Component: Core Core: HLIL Effort: High IL Optimization Impact: High
Dominant language
C++
Stars
1.3k
Forks
298
Avg merge
5d 5h
Merged PRs (30d)
19

Description

I have the follow HLIL:

int64_t keystore_open@1714(int64_t arg1, int32_t* arg2, char* arg3, int16_t* arg4)

00001750  uint8_t buf[1360]
00001750  memset@41c8(dst: &buf, c: 0, sz: 0x550)
00001760  int64_t __saved_x29
00001760  int64_t x0
00001760  int64_t x8
00001760  int64_t x29_1
00001760  x0, x8, x29_1 = keystore_sys_open@9ec("keystore", &__saved_x29)
00001764  int64_t r
00001764  if (arg1 u> 4)
00001778      printf@2da0("KEYSTORE: KEY_ID %d not in keyst…")
0000177c  ret_error@177c:
0000177c      r = -1
00001788  else
00001788      char* const x0_2
00001788      int64_t x19_1
00001788      if (zx.d(*arg4) u<= 0x100)
000017a0          int64_t var_558 = 0x550
000017a8          if (x0:0.d s<= 0)
000017a8              goto ret_error@177c
000017b4          int64_t x20_1 = sx.q(x0:0.d)
000017bc          sub_a44(x8, x20_1, &var_558, arg1, x29_1)
000017c4          keystore_sys_close@b04(x20_1, x29_1)
000017cc          x19_1 = arg1 * 0x110
000017d8          void* x20_2 = x19_1 + &buf
00001788          if (*(x19_1 + &buf) != 0x59454b)
00001848              x0_2 = "KEYSTORE: bad keystore magic\n"
000017ec          else
000017ec              uint64_t x2 = zx.q(zx.d(*arg4))
000017f0              uint64_t x1_5 = zx.q(zx.d(*(x20_2 + 0xc)))
000017f4              *arg2 = *(x20_2 + 8)
0000183c              if (x2:0.d u< x1_5:0.d)
0000183c                  printf@2da0("KEYSTORE: bad key len slot:%d re…")
00001840                  goto ret_error@177c
0000180c              memcpy@4174(dst: arg3, src: x20_2 + 0x10, sz: x1_5 & 0xffff)
00001814              *arg4 = *(x20_2 + 0xc)
00001818              r = 0
00001794      else
00001794          x0_2 = "KEYSTORE: key too big\n"
00001798      if (zx.d(*arg4) u> 0x100 || ((zx.d(*arg4) u<= 0x100 && x0:0.d s> 0) && *(x19_1 + &buf) != 0x59454b))
00001798          printf@2da0(x0_2)
0000179c          goto ret_error@177c
00001830  return r

Specifically, the placement of the error path label:

0000177c  ret_error@177c:
0000177c      r = -1

seems surprising. I would expect this to be relocated to near the end of the function. Happy to provide binaries/instructions/debug reprots/whatever to help.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

The HLIL example centers on keystore_open@1714 and the ret_error@177c label; begin by reproducing that output from the relevant binary and inspect how HLIL places this error path. Confirm completion by verifying that the generated error label is positioned as expected near the function's end while preserving the shown control flow.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
reverse-engineering
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.