Vector35 / binaryninja-api

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

Defining a structure causes analysis results to be incorrect #3823

Open pr0xy-t opened 1 year ago

pr0xy-t commented 1 year ago

Version and Platform (required):

Bug Description: Defining a structure causes analysis results to be incorrect. A wrong value is propagated to a variable.

Steps To Reproduce: In "Disassembly window", at 0x1167 address, the eax register should be assigned 0x1 , but the following procedure will result in 0x61 in the eax register. Also, (as a result,) in "Pseudo C window",at 0x117b address, the argument of printf becomes 0x61 not 1. files.zip

  1. Analyze a.out. 1.1 As shown in the below pic, the result of the analysis is correct before defining the structure. Screenshot from 2023-01-24 13-43-07

  2. Define structure S as follows

    struct S __packed
    {
    char buf0;
    __padding char _1[3];
    int32_t buf1;
    int32_t buf2;
    };
  3. In the pseudo c window, move the mouse cursor over the variable name (ver_14) at 0x1155, press the y key, and change the type of variable ver_14 to struct S s. Screenshot from 2023-01-24 13-54-25

  4. As show in the below pic, in "Disassembly window", at 0x1167 address, the eax register is assigned 0x61. Also, in "Pseudo C window", at 0x117b, the argument of printf becomes 0x61 not 1 . Screenshot from 2023-01-24 14-00-13

pr0xy-t commented 1 year ago

I will update this issue as soon as I find the minimum occurrence conditions or causes of bugs.

CouleeApps commented 1 year ago

I can confirm this is happening with the example you sent. It seems like the creation of the structure is causing the stack contents to become undetermined, except for the first var set in the structure. Then, the load seems like it is falling back to the start of the structure in the stack lookup. Good find.

CouleeApps commented 1 year ago

Before struct:

>>> [(i, current_function.get_stack_contents_at(here, i, 1)) for i in [-0x8, -0xc, -0x10, -0x14]]
[(-8, <entry rbp>), (-12, <const 0x2>), (-16, <const 0x1>), (-20, <const 0x61>)]

After struct:

>>> [(i, current_function.get_stack_contents_at(here, i, 1)) for i in [-0x8, -0xc, -0x10, -0x14]]
[(-8, <entry rbp>), (-12, <undetermined>), (-16, <undetermined>), (-20, <const 0x61>)]
CouleeApps commented 1 year ago

So the real bug here is that the store of 0x61 is being propagated at all. Generally speaking, we don't yet support dataflow for larger types (like this structure) through the stack, and the fact that it's not all undetermined is a bug. In the future, we have plans for larger sized dataflow (for things like vector types) but the way this is working right now is a bug.