fkie-cad / dewolf

A research decompiler implemented as a Binary Ninja plugin.
GNU Lesser General Public License v2.1
172 stars 9 forks source link

[Lifter] Call parameters lifting is incorrect #319

Closed mari-mari closed 1 year ago

mari-mari commented 1 year ago

What happened?

The instruction

mem#2 = __libc_start_main(main: main, argc: rsi#1, ubp_av: rdx#1, init: __libc_csu_init, fini: __libc_csu_fini, rtld_fini: r9#1, stack_end: var_10#1) @ mem#1

is decompiled to:

__libc_start_main(main, var_1, /* envp) */ &var_2, __libc_csu_init, /* ubp_av */ __libc_csu_fini, /* init)() */ arg3, /* fini)() */ &var_3);

The issue occurs in _lift_call_parameter_names. The method tries to parse parameter names from string ')(int32_t (* main)(int32_t argc, char** argv, char** envp), int32_t argc, char** ubp_av, void (* init)(), void (* fini)(), void (* rtld_fini)(), void* stack_end) __noreturn'.

But in this case the first argument is the function pointer itself (main). Hence, the string above contains not only the signature of __libc_start_main, but also embedded signature of main method. This makes the parsing incorrect.

The parsing should be fixed. Alternatively, it should be checked, if Binary Ninja now provides any API methods to retrieve those parameter names. If this is the case, these methods should be used instead of the string parsing.

How to reproduce?

python decompile.py forkingserver _start forkingserver.zip

Affected Binary Ninja Version(s)

3.4.4271

NeoQuix commented 1 year ago

After looking at bit into the API, I found the class FunctionType which has a list of FunctionParameters where the correct names are.

It's a bit ugly to get there but the following works:

@staticmethod
def _lift_call_parameter_names(instruction: mediumlevelil.MediumLevelILCall) -> List[str]:
    """Lift parameter names of call by iterating over the function parameters where the call is pointing to (if available)"""
    if instruction.dest.expr_type is None or not isinstance(instruction.dest.expr_type, PointerType) or \
    not isinstance(instruction.dest.expr_type.target, FunctionType):
        return []
    return [param.name for param in instruction.dest.expr_type.target.parameters]

This code results in:

long _start(long arg1, long arg2, void() * arg3) {
    void var_2;
    long var_3;
    var_3 = var_0;
    __libc_start_main(/* main */ main, var_1, /* ubp_av */ &var_2, /* init */ __libc_csu_init, /* fini */ __libc_csu_fini, /* rtld_fini */ arg3, /* stack_end */ &var_3);
}

which is exactly like bninja has it.

NeoQuix commented 1 year ago

/cib

github-actions[bot] commented 1 year ago

Branch issue-319-_Lifter_Call_parameters_lifting_is_incorrect created!