JonathanSalwan / Triton

Triton is a dynamic binary analysis library. Build your own program analysis tools, automate your reverse engineering, perform software verification or just emulate code.
https://triton-library.github.io
Apache License 2.0
3.5k stars 533 forks source link

Fix call semantics #1182

Closed cnheitman closed 2 years ago

cnheitman commented 2 years ago

Hey @JonathanSalwan o/ There is an issue with the semantics of call. It has an odd behavoir that depends on whether a GET_CONCRETE_MEMORY_VALUE callback is set. The following snippet exposes the issue:

from triton import *

def test_call(add_callback):
    def get_concrete_memory_value_hook(ctx, mem):
        print(f'\tInside callback, mem: {mem}')

    ctx = TritonContext(ARCH.X86_64)

    rsp = 0xeffffd80

    ctx.setConcreteMemoryValue(MemoryAccess(rsp + 0x48, 8), 0x41c8e0)
    ctx.setConcreteMemoryValue(MemoryAccess(rsp + 0x50, 8), 0x41c910)

    ctx.setConcreteRegisterValue(ctx.registers.rsp, 0xeffffd80)

    print(f"\t[rsp + 0x48]([{rsp + 0x48:#x}]) = {ctx.getConcreteMemoryValue(MemoryAccess(rsp + 0x48, 8)):#x}")
    print(f"\t[rsp + 0x50]([{rsp + 0x50:#x}]) = {ctx.getConcreteMemoryValue(MemoryAccess(rsp + 0x50, 8)):#x}")

    if add_callback:
        ctx.addCallback(CALLBACK.GET_CONCRETE_MEMORY_VALUE, get_concrete_memory_value_hook)

    inst = Instruction(0x41C690, b"\xff\x54\x24\x50")   # call qword ptr [rsp + 0x50]

    ctx.processing(inst)

    print(f'\t{inst}')

    print(f'\trip: {ctx.getConcreteRegisterValue(ctx.registers.rip):x}')

print('[+] Without setting the callback: ')
test_call(False)

print('[+] Setting the callback: ')
test_call(True)

This is the output:

[+] Without setting the callback:
    [rsp + 0x48]([0xeffffdc8]) = 0x41c8e0
    [rsp + 0x50]([0xeffffdd0]) = 0x41c910
    0x41c690: call qword ptr [rsp + 0x50]
    rip: 41c910
[+] Setting the callback:
    [rsp + 0x48]([0xeffffdc8]) = 0x41c8e0
    [rsp + 0x50]([0xeffffdd0]) = 0x41c910
    Inside callback, mem: [@0xeffffdd0]:64 bv[63..0]
    0x41c690: call qword ptr [rsp + 0x50]
    rip: 41c8e0

The address of mem access varies when the callback is set (the first is the correct one, which can be determine by the value that ends up in rip and not the value that is shown by the callback). The reason why it happens when the callback is set is because after calling the callback, processCallback sets the leaAst of the memory access. The first thing done in call_s is aligning the stack to reserve space for holding the return address. After that the AST for the memory access is computed. This AST is affected by what the processCallback does. When the callback is set it takes into account the stack alignment.

This PR fixes this (by reserving the space after building the AST of the mem access in call_s) for this but I wanted to double check with you that I'm not missing anything here.

JonathanSalwan commented 2 years ago

Hey ameo. Nice catch! The patch looks good to me. I've also checked where we also use alignSubStack_s, for example in the push_s, and it looks we did it correctly.