annihilatorq / shadow_syscall

windows syscalls with a single line and a high level of abstraction. has modern cpp20 wrappers and utilities, range-based DLL and export enumeration, wrapper around kuser_shared_data. supported compilers: clang, gcc and msvc
Apache License 2.0
111 stars 11 forks source link

ScyllaHide #1

Closed po0p closed 1 week ago

po0p commented 2 weeks ago

ScyllaHide breaks the library, on almost any preset.

// Syscall ID is at an offset of 4 bytes from the specified address.
__forceinline std::uint32_t parse_syscall_id( std::uintptr_t export_address ) const {
    return *reinterpret_cast< std::uint32_t* >( static_cast< std::uintptr_t >( export_address + 4 ) );
}

This is kinda invalid, because if scyllahide/any EDR solution hooks the Nt/Zw function, the assembly is broken as syscall id is no longer there. (because detour asm usually takes more than 6 bytes), but you will still return me a syscall id of something like 2324345345 Possible solutions:

  1. check1: all syscalls (to my knowledge at least, starting from win10 x64) start with mov r10, rcx. x64dbg_nFuxYljmus (see also this: https://github.com/am0nsec/HellsGate/blob/1d860c0734c0e35a2f026d9a04856ded19dfdf31/HellsGate/main.c#L146) Can do 2 checks here: if sum of first 3 bytes of function == 0x1a8 (4c + 8b + d1), or can just compare directly ( *reinterpret_cast<uint8_t*>(export_address) -> export_address+1 -> export_address +2

  2. check 2: since you already parse the exports, can do something like

    size_t fun_size = fori (0-64) if (fun[i] == 0xc3//ret asm// return i+1)

    when size of function is aquired, you can also walk the function to check if it has 2 consecutive bytes of 0x0f and 0x05(syscall instruction basically). Ntdll exports some functions that do not actually result in syscalls: x64dbg_nI9Utdh3Xu x64dbg_Qx693oSclC x64dbg_QyY1OZHMAC

and this tech can be useful for implementing/adding indirect syscall technique

а так норм либа

bambinorest1 commented 1 week ago

I think you need to parse syscall id from dll loaded from disk. And maybe you should add 8 bytes for hash to avoid collision

po0p commented 1 week ago

I think you need to parse syscall id from dll loaded from disk. And maybe you should add 8 bytes for hash to avoid collision

There are many ways to parse syscall ids, parsing from disk via ntopenfile+ntmapviewofsection will not work in this specific case. (see this https://github.com/x64dbg/ScyllaHide/blob/baa5c8e853ace2bee752631f27fdfe5b271d92f6/HookLibrary/HookedFunctions.cpp#L1159) Also, i believe that with original purpose of this library, @annihilatorq should just signal somehow that syscall id is invalid when calling Call, parsing syscall ids is part of Hell's Gate and similar implementations; which was not really target of the shadow_syscall. Correct me if im wrong.

bambinorest1 commented 1 week ago

How do you propose to parse then to avoid reading hooked functions and read correct syscall id? Before this I thought that reading from disk was the best option)

po0p commented 1 week ago

How do you propose to parse then to avoid reading hooked functions and read correct syscall id? Before this I thought that reading from disk was the best option)

I am not proposing to read correct syscalls id, I am proposing to return something indicating that parsing failed. It is better than trying to execute syscall(215981295812958125); Many ways to obtain correct syscalls, starting from just hardcoding the table for 100500 windows versions into the binary

annihilatorq commented 1 week ago

Also, i believe that with original purpose of this library, @annihilatorq should just signal somehow that syscall id is invalid when calling Call

Hope this solution fits, please review and test. If everything works as expected, I'll close the issue.

// The function returns a pair: the first element is the
// NTSTATUS, and the second is an optional error code.
//
// Assumed to be invoked with a structured binding to get
// any error information via std::optional
auto [status, error] = shadowsyscall<NTSTATUS>(
    "NtCreateThreadEx",
    &process,
    THREAD_ALL_ACCESS,
    NULL,
    current_process,
    static_cast< LPTHREAD_START_ROUTINE >( start_routine ),
    0,
    FALSE,
    NULL,
    NULL,
    NULL,
    0
);

if ( error ) {
    std::cerr << "Error code is: " << *error << "\n";

    if ( *error == shadow::errc::ssn_not_found ) {
        std::cerr << "Failed to resolve System Service Number\n";
    }
}
else {
    std::cout << "NtCreateThreadEx call status: 0x" << std::hex << status << '\n';
}

// Can be called and should be used when error
// information is not required. Default "Ty"
// value will be returned and no syscall is executed.
auto other_status = shadowsyscall<NTSTATUS>( "NtCreateThreadEx" );

In the future, it is planned to add functionality to override syscall index parsing Every PR is absolutely welcome though.

po0p commented 1 week ago

Hope this solution fits, please review and test. If everything works as expected, I'll close the issue.

Smoll test, everything works as expected. No hooks: consoleapp_sax07v6bDY

Invalid syscall id (NtClose hooked): explorer_LPQp1uUU0O

Looks pretty fine to me! Im not very good with c++, maybe you can use std::expected wrapper over call result and errcode, maybe its easier than having own struct call_result_t?

it is planned to add functionality to override syscall index parsing

Good luck with that! I feel a little bit weird about it, because that way you will probably have to rely on static table, while right now ssn fetching is fully dynamic , which may be an advantage in some situations. Why static table? Well, you see, any edr worth their price wont like that you have 2 ntdlls loaded in the process ;) But, anyway, not related to this issue. спс за быстрый фикс!

annihilatorq commented 1 week ago

Im not very good with c++, maybe you can use std::expected wrapper over call result and errcode, maybe its easier than having own struct call_result_t?

Of course this would be the best and most concise solution, but in this case we would have to give up cpp20 users, which might not be a justifiable sacrifice for the beauty of std::expected. I would happily support standards below cpp20 as well, if it weren't for the need to use consteval in hash constructors

Thanks for your help in improving the repository @po0p! тебе спасибо :^)

annihilatorq commented 1 week ago

In the future, it is planned to add functionality to override syscall index parsing

Although this issue was closed earlier, I wanted to update you that the discussed feature has now been implemented in commit [9c804b3]