Extra arguments added to function in THUMB

[raminri]

Version and Platform (required):

• Binary Ninja Version: 4.3.6844
• OS: Windows
• OS Version: 10
• CPU Architecture: x64

Bug Description:

I often see this pattern come up when looking at THUMB code:

push {r0, r1, r2, r3, r4, lr}
movs r4, #0
str r4, [sp, #4]
str r4, [sp, #8]
...
add sp, #0x10
pop {r4, pc}

There are 6 registers that are saved to the stack in the function prologue, but only 2 are actually restored in the epilogue. The r0/r1/r2/r3 stack slots are used as variables instead of actually being used to save/restore the registers. I guess the compiler may do this as an optimization to remove a sub sp, X instruction.

Binja does not handle this pattern well, and it leads to code that looks like this in HLIL:

int32_t func(int32_t arg1, int32_t arg2, int32_t arg3, int32_t arg4)
    int32_t var_c = arg4
    int32_t var_10 = arg3
    int32_t var_14 = arg2
    var_14 = 0
    var_10 = 0
    ...

The function is thought to have 4 arguments when it really only has 1.

Steps To Reproduce:

Can't share the binary that I encountered this in, but I did my best to create a minimal example for this (which may be a little contrived).

Please provide all steps required to reproduce the behavior:

1. Create new window
2. Paste in following bytes: 1fb505460024019401a800f005f8019c04eb050004b010bd052101607047
3. Make thumb2 function
4. See that sub_0 is shown to have 2 arguments

Expected Behavior:

The function should not have any arguments appended if the stack variable the argument is written to is being reused as a regular variable.

A possible heuristic for this could be comparing the push/pop instructions of the function and considering any non-restored registers to not be arguments.

[galenbwill]

Can you re-check your private binary in 5.0 stable or current dev?

On 5.1.7314-dev, I'm seeing this HLIL for your example:

00000000    int32_t sub_0(int32_t arg1, int32_t arg2)

00000000        int32_t r3
00000000        int32_t var_c = r3
00000000        int32_t r2
00000000        int32_t var_10 = r2
00000000        int32_t var_14 = arg2
00000000        int32_t var_18 = arg1
00000006        var_14 = 0
0000000a        sub_18(&var_14)
00000016        return var_14 + arg1


00000018    void sub_18(int32_t* arg1)

0000001a        *arg1 = 5

[raminri]

Yes that is the incorrect HLIL. sub_0 should only have 1 argument, not 2.
r1 is only pushed onto the stack to create stack space, and is not an actual argument.

I've found another binary that demonstrates this issue which I can share.
It has been uploaded as kind ocean jumps quietly.
Load the binary as thumb2 with base address 0x100c00 then view function sub_10c382.

0010c382    int32_t sub_10c382(int32_t arg1, int32_t arg2, int32_t arg3, int32_t arg4)

0010c382        int32_t var_8 = arg4
0010c386        var_8.b = 0xff
0010c394        sub_12de70(0, 0x138, &var_8, 1)
0010c394        
0010c39e        if (zx.d(var_8.b) == 0xff)
0010c3a6            return 1
0010c3a6        
0010c3a2        return 0

This function should be taking no arguments, but is shown to take 4 due to the compiler using the push instruction to allocate stack space:

0010c382  08b5       push    {r3, lr} {var_8} {var_4}
0010c384  ff20       movs    r0, #0xff
0010c386  8df80000   strb    r0, [sp] {var_8}  {0xff}
0010c38a  0123       movs    r3, #1
0010c38c  6a46       mov     r2, sp {var_8}
0010c38e  4ff49c71   mov     r1, #0x138
0010c392  0020       movs    r0, #0
0010c394  21f06cfd   bl      #sub_12de70
0010c398  9df80000   ldrb    r0, [sp] {var_8}
0010c39c  ff28       cmp     r0, #0xff
0010c39e  01d0       beq     #0x10c3a4

0010c3a0  0020       movs    r0, #0
0010c3a2  08bd       pop     {r3, pc} {var_8} {var_4}

0010c3a4  0120       movs    r0, #1
0010c3a6  08bd       pop     {r3, pc} {var_8} {var_4}

The incorrect 4 arguments count propagates to the caller as well, and results in very ugly looking HLIL with multiple returns when a broken function is called:

0010b89e        r0, r1, r2, r3 = sub_10c4f8(arg1, arg2, arg3, arg4)

The address I've given isn't the only example of this issue in the binary, it is littered with other cases. Anywhere where a function is returning multiple values indicates that this issue is present.

[galenbwill]

Ah, I see what you mean. I suspect this is an analysis problem, not a lifting problem, but someone else will need to take a look at it, which is why I'm marking it Awaiting Triage again. (This looks like a @rssor thing.)