Vector35 / Vector35/binaryninja-api

Binja incorrectly translating ILP32 tailcall pattern

Open
#7,930 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Effort: Low Impact: Low
Dominant language
C++
Stars
1.3k
Forks
298
Avg merge
5d 5h
Merged PRs (30d)
19

Description

Bug Description:
Binja incorrectly translating AArch64 ILP32 tailcall pattern as a result of 32-bit load into w17 and jump through x17.

00401dc0    uint32_t strtoul(char const* str, char** endptr, int32_t base)
00401dc0  adrp    x16, getspnam
00401dc4  ldr     w17, [x16, #0x10c]  {strtoul}
00401dc8  add     w16, w16, #0x10c  {strtoul}
❓00401dcc  br      x17

Steps To Reproduce:
Please provide all steps required to reproduce the behavior:

  1. Open the attached binary in Binja 5.3.9025 or later
  2. Navigate to strtoul in .plt
  3. Observe the tag at 0x401dcc on the jump(zx.q(*getaddrinfo))

Expected Behavior:
Translate correctly resulting in x17 being a resolved constant value

Binary:
victory spring enters valuably

Additional Information:

This hack allows the PLT branches to the extern region to work fine, but this isn't a good solution and I think we need to fix this in core.

diff --git a/arch/arm64/il.cpp b/arch/arm64/il.cpp
index f02cdd2d..ab6ff084 100644
--- a/arch/arm64/il.cpp
+++ b/arch/arm64/il.cpp
@@ -905,8 +905,11 @@ static void LoadStoreOperand(LowLevelILFunction& il, bool load,
                            ILSETREG_O(operand1, il.Operand(1, il.Load(load_store_sz, ILREG_O(operand2)))));
                        break;
                case MEM_OFFSET:
-                       if (!load_store_sz)
-                               load_store_sz = REGSZ_O(operand1);
+                       if ((operand1.reg[0] >= REG_W0 && operand1.reg[0] <= REG_WSP) || (operand1.reg[0] >= REG_S0 && operand1.reg[0] <= REG_S31))
+                       {
+                               BNRegisterInfo regInfo = il.GetArchitecture()->GetRegisterInfo(operand1.reg[0]);
+                               operand1.reg[0] = (Register)regInfo.fullWidthRegister;
+                       }

                        // operand1.reg = [operand2.reg + operand2.imm]
                        if (IMM_O(operand2) == 0)

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

Start in arch/arm64/il.cpp at LoadStoreOperand and inspect how the AArch64 IL handles the 32-bit load into w17 followed by a jump through x17. Reproduce with the attached binary in Binja, navigate to strtoul in .plt, and confirm the 0x401dcc jump is translated with x17 as a resolved constant rather than an incorrect tag.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
reverse-engineering
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.