Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thumb2: Code not recognized #4305

Closed
joelreymont opened this issue May 11, 2023 · 6 comments
Closed

Thumb2: Code not recognized #4305

joelreymont opened this issue May 11, 2023 · 6 comments
Labels
Arch: ARM/Thumb Issues with the ARM/Thumb architecture plugin Component: Architecture Issue needs changes to an architecture plugin Impact: High Issue adds or blocks important functionality State: Duplicate Issue is a duplicate of another issue

Comments

@joelreymont
Copy link

joelreymont commented May 11, 2023

Version and Platform (required):

  • Binary Ninja Version: 3.4.4251-dev, 3f761e0a
  • OS: macos
  • OS Version: 13.3
  • CPU Architecture: arm64

Note the jump to 0x4d10 which has code

000004d00  uint32_t sub_4d00(int32_t arg1, int32_t arg2, uint32_t arg3, uint32_t arg4)

000004d00  7bb9       cbnz    r3, #0x4d22

000004d02  72b9       cbnz    r2, #0x4d22

000004d04  0029       cmp     r1, #0
000004d06  bebf       ittt    lt
000004d08  0020       movs    r0, #0  {0x0}
000004d0a  4ff00041   mov     r1, #0x80000000
000004d0e  06e0       b       #0x4d1e  {data_4d10}

000004d10  data_4d10:
000004d10                                                  08 bf 00 28 1c bf 6f f0 00 41 4f f0 ff 30                        ...(..o..AO..0

000004d1e  fff723bd   b       #sub_4768

that should look like this

ROM:00004D10                 IT EQ
ROM:00004D12                 CMPEQ           R0, #0
ROM:00004D14                 ITT NE
ROM:00004D16                 MOVNE           R1, #0x7FFFFFFF ; a1
ROM:00004D1A                 MOVNE.W         R0, #0xFFFFFFFF ; a1

Also, undefining and redefining the function does not create code at that location. How do I force BN to create code here?

@joelreymont
Copy link
Author

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

@fuzyll fuzyll added Component: Architecture Issue needs changes to an architecture plugin Arch: ARM/Thumb Issues with the ARM/Thumb architecture plugin labels May 19, 2023
@fuzyll
Copy link
Contributor

fuzyll commented May 19, 2023

I'm a little bit confused on this one. Unless we are disassembling the branch instruction at 0x4d0e incorrectly, our disassembly output should be correct. Marking it up with the label is the part that's misleading.

I can get the bytes starting at 0x4d10 to be disassembled by right-clicking and saying Make Function at this Address -> thumb2 -> linux-thumb2 (and the output roughly matches what you have here). I don't see any cross-references to this location, though, so it's not clear whether this is ever executed or not.

You also have an overlapping function defined just above this one (looks like it was maybe created with thumb2 instead of linux-thumb2, based on my playing around with things). I don't think that's at all involved, just wanted to point it out.

@xusheng6
Copy link
Member

We do not create a function at data_4d10 because there is no indication that there is code at that address. In other words, there is no control-flow (e.g., calls, jumps) getting into it.

If you wish to create a function there, right click and select "Create Function at Address" -> "thumb2" -> "linux-thumb2".

I am closing this because things work as our expectation. Please feel free to re-open it if you have more info, e.g., there is actually a control-flow leading to data_4d10 but we still miss it.

@joelreymont
Copy link
Author

That code is part of a function but BN is not recognizing it as such. See below

ROM:00004D00 ; =============== S U B R O U T I N E =======================================
ROM:00004D00
ROM:00004D00
ROM:00004D00 sub_4D00                                ; CODE XREF: sub_A720+6C↓p
ROM:00004D00
ROM:00004D00 var_10          = -0x10
ROM:00004D00 var_C           = -0xC
ROM:00004D00 var_8           = -8
ROM:00004D00
ROM:00004D00 ; FUNCTION CHUNK AT ROM:00004768 SIZE 00000002 BYTES
ROM:00004D00
ROM:00004D00                 CBNZ            R3, loc_4D22
ROM:00004D02                 CBNZ            R2, loc_4D22
ROM:00004D04                 CMP             R1, #0
ROM:00004D06                 ITTT LT
ROM:00004D08                 MOVLT           R0, #0
ROM:00004D0A                 MOVLT.W         R1, #0x80000000
ROM:00004D0E                 BLT             loc_4D1E
ROM:00004D10                 IT EQ
ROM:00004D12                 CMPEQ           R0, #0
ROM:00004D14                 ITT NE
ROM:00004D16                 MOVNE           R1, #0x7FFFFFFF ; a1
ROM:00004D1A                 MOVNE.W         R0, #0xFFFFFFFF ; a1
ROM:00004D1E
ROM:00004D1E loc_4D1E                                ; CODE XREF: sub_4D00+E↑j
ROM:00004D1E                 B.W             locret_4768
ROM:00004D22 ; ---------------------------------------------------------------------------
ROM:00004D22
ROM:00004D22 loc_4D22                                ; CODE XREF: sub_4D00↑j
ROM:00004D22                                         ; sub_4D00+2↑j
ROM:00004D22                 SUB.W           R12, SP, #8
ROM:00004D26                 STRD.W          R12, LR, [SP,#var_10]! ; a3
ROM:00004D2A                 CMP             R1, #0
ROM:00004D2C                 BLT             loc_4D42
ROM:00004D2E                 CMP             R3, #0
ROM:00004D30                 BLT             loc_4D68
ROM:00004D32                 BL              __udivmoddi4
ROM:00004D36                 LDR.W           LR, [SP,#0x10+var_C]
ROM:00004D3A                 LDRD.W          R2, R3, [SP,#0x10+var_8] ; a2
ROM:00004D3E                 ADD             SP, SP, #0x10
ROM:00004D40                 BX              LR      ; a3
ROM:00004D42 ; ---------------------------------------------------------------------------
ROM:00004D42
ROM:00004D42 loc_4D42                                ; CODE XREF: sub_4D00+2C↑j
ROM:00004D42                 NEGS            R0, R0  ; a1
ROM:00004D44                 SBC.W           R1, R1, R1,LSL#1 ; a1
ROM:00004D48                 CMP             R3, #0
ROM:00004D4A                 BLT             loc_4D84
ROM:00004D4C                 BL              __udivmoddi4
ROM:00004D50                 LDR.W           LR, [SP,#0x10+var_C]
ROM:00004D54                 LDRD.W          R2, R3, [SP,#0x10+var_8]
ROM:00004D58                 ADD             SP, SP, #0x10
ROM:00004D5A                 NEGS            R0, R0  ; a1
ROM:00004D5C                 SBC.W           R1, R1, R1,LSL#1 ; a1
ROM:00004D60                 NEGS            R2, R2
ROM:00004D62                 SBC.W           R3, R3, R3,LSL#1
ROM:00004D66                 BX              LR      ; a3
ROM:00004D68 ; ---------------------------------------------------------------------------
ROM:00004D68
ROM:00004D68 loc_4D68                                ; CODE XREF: sub_4D00+30↑j
ROM:00004D68                 NEGS            R2, R2  ; a2
ROM:00004D6A                 SBC.W           R3, R3, R3,LSL#1 ; a2
ROM:00004D6E                 BL              __udivmoddi4
ROM:00004D72                 LDR.W           LR, [SP,#0x10+var_C]
ROM:00004D76                 LDRD.W          R2, R3, [SP,#0x10+var_8]
ROM:00004D7A                 ADD             SP, SP, #0x10
ROM:00004D7C                 NEGS            R0, R0
ROM:00004D7E                 SBC.W           R1, R1, R1,LSL#1
ROM:00004D82                 BX              LR      ; a3
ROM:00004D84 ; ---------------------------------------------------------------------------
ROM:00004D84
ROM:00004D84 loc_4D84                                ; CODE XREF: sub_4D00+4A↑j
ROM:00004D84                 NEGS            R2, R2  ; a2
ROM:00004D86                 SBC.W           R3, R3, R3,LSL#1 ; a2
ROM:00004D8A                 BL              __udivmoddi4
ROM:00004D8E                 LDR.W           LR, [SP,#0x10+var_C]
ROM:00004D92                 LDRD.W          R2, R3, [SP,#0x10+var_8]
ROM:00004D96                 ADD             SP, SP, #0x10
ROM:00004D98                 NEGS            R2, R2
ROM:00004D9A                 SBC.W           R3, R3, R3,LSL#1
ROM:00004D9E                 BX              LR
ROM:00004D9E ; End of function sub_4D00

@fuzyll
Copy link
Contributor

fuzyll commented May 22, 2023

Yeah, so the issue is that we believe the branch at 0x4d0e is unconditional. Thus, we don't disassemble the code below it, which is the correct behavior.

The reason we don't believe this is conditional appears to be because we've completely ignored the ittt lt at 0x4d06. The next three instructions (which includes the b) should be marked with the less-than condition, but instead they aren't. So, this ticket definitely should not have been closed - this appears to be a very legitimate bug in our ARM disassembly.

@fuzyll fuzyll reopened this May 22, 2023
@fuzyll fuzyll added the Impact: High Issue adds or blocks important functionality label May 22, 2023
@fuzyll
Copy link
Contributor

fuzyll commented May 22, 2023

Actually, sorry, I'm re-closing this as a duplicate of #3976. The issue was already triaged then (the code in that ticket is the same exact thing, too), I just didn't know about it. We should still address this problem, I just want to make sure we've only got a single ticket to track it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: ARM/Thumb Issues with the ARM/Thumb architecture plugin Component: Architecture Issue needs changes to an architecture plugin Impact: High Issue adds or blocks important functionality State: Duplicate Issue is a duplicate of another issue
Projects
None yet
Development

No branches or pull requests

3 participants