Skip to content

Commit

Permalink
Detours hooking is incompatible with AnsiCon.
Browse files Browse the repository at this point in the history
Detours effectively gives Clink's hooks the lowest precedence.  Clink's
hooks need to have high precedence in order to successfully detect when
CMD prints the prompt.

This reverts Clink to use IAT hooking by default.  And it goes further,
using IAT hooking even for ReadConsoleW, which had always used address
hooking.  Originally everything used address hooking, and Martin slowly
converted things to use IAT hooks, but never converted ReadConsoleW.

Clink can be forced to use Detours hooking via `clink inject --detours`.
But don't use `--detours` unless it's the only way to get Clink working.
  • Loading branch information
chrisant996 committed Sep 24, 2021
1 parent a986b25 commit ba71118
Show file tree
Hide file tree
Showing 9 changed files with 279 additions and 40 deletions.
31 changes: 24 additions & 7 deletions clink/app/src/host/host_cmd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ func_SetEnvironmentVariableW_t __Real_SetEnvironmentVariableW = SetEnvironmentVa
func_WriteConsoleW_t __Real_WriteConsoleW = WriteConsoleW;
func_ReadConsoleW_t __Real_ReadConsoleW = ReadConsoleW;
func_GetEnvironmentVariableW_t __Real_GetEnvironmentVariableW = GetEnvironmentVariableW;
static const char s_kernel_module[] = "kernel32.dll";

//------------------------------------------------------------------------------
extern bool is_force_reload_scripts();
Expand All @@ -60,6 +59,19 @@ static setting_str g_admin_title_prefix(



//------------------------------------------------------------------------------
static hook_type get_hook_type()
{
static hook_type s_hook_type = app_context::get()->is_detours() ? detour : iat;
return s_hook_type;
}
static const char* get_kernel_module()
{
return (get_hook_type() == iat) ? nullptr : "kernel32.dll";
}



//------------------------------------------------------------------------------
static bool get_mui_string(int id, wstr_base& out)
{
Expand Down Expand Up @@ -244,20 +256,22 @@ int host_cmd::validate()
bool host_cmd::initialise()
{
hook_setter hooks;
hook_type type = get_hook_type();
const char* module = get_kernel_module();

// Hook the setting of the 'prompt' environment variable so we can tag
// it and detect command entry via a write hook.
tag_prompt();
if (!hooks.attach(s_kernel_module, "SetEnvironmentVariableW", &host_cmd::set_env_var, &__Real_SetEnvironmentVariableW))
if (!hooks.attach(type, module, "SetEnvironmentVariableW", &host_cmd::set_env_var, &__Real_SetEnvironmentVariableW))
return false;
if (!hooks.attach(s_kernel_module, "WriteConsoleW", &host_cmd::write_console, &__Real_WriteConsoleW))
if (!hooks.attach(type, module, "WriteConsoleW", &host_cmd::write_console, &__Real_WriteConsoleW))
return false;

// Set a trap to get a callback when cmd.exe fetches PROMPT environment
// variable. GetEnvironmentVariableW is always called before displaying the
// prompt string, so it's a reliable spot to hook regardless how injection
// is initiated (AutoRun, command line, etc).
if (!hooks.attach(s_kernel_module, "GetEnvironmentVariableW", &host_cmd::get_env_var, &__Real_GetEnvironmentVariableW))
if (!hooks.attach(type, module, "GetEnvironmentVariableW", &host_cmd::get_env_var, &__Real_GetEnvironmentVariableW))
return false;

return hooks.commit();
Expand Down Expand Up @@ -563,7 +577,7 @@ DWORD WINAPI host_cmd::get_env_var(LPCWSTR lpName, LPWSTR lpBuffer, DWORD nSize)
s_initialised_system = true;

hook_setter unhook;
unhook.detach(&__Real_GetEnvironmentVariableW, get_env_var);
unhook.detach(get_hook_type(), get_kernel_module(), "GetEnvironmentVariableW", &__Real_GetEnvironmentVariableW, get_env_var);
unhook.commit();

host_cmd::get()->initialise_system();
Expand Down Expand Up @@ -606,10 +620,13 @@ DWORD WINAPI host_cmd::format_message(DWORD flags, LPCVOID source, DWORD message
bool host_cmd::initialise_system()
{
{
hook_type type = get_hook_type();
const char* module = get_kernel_module();

// ReadConsoleW is required.
{
hook_setter hooks;
hooks.attach(s_kernel_module, "ReadConsoleW", &host_cmd::read_console, &__Real_ReadConsoleW);
hooks.attach(type, module, "ReadConsoleW", &host_cmd::read_console, &__Real_ReadConsoleW);
if (!hooks.commit())
return false;
}
Expand All @@ -619,7 +636,7 @@ bool host_cmd::initialise_system()
// prefix, but ignore failure since it's just a minor convenience.
{
hook_setter hooks;
hooks.add(s_kernel_module, "FormatMessageW", &host_cmd::format_message, &__imp_FormatMessageW);
hooks.add(type, module, "FormatMessageW", &host_cmd::format_message, &__imp_FormatMessageW);
hooks.commit();
}
#endif
Expand Down
9 changes: 4 additions & 5 deletions clink/app/src/loader/inject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,7 @@ int inject(int argc, char** argv)
{ "pid", required_argument, nullptr, 'd' },
{ "nolog", no_argument, nullptr, 'l' },
{ "autorun", no_argument, nullptr, '_' },
{ "detours", no_argument, nullptr, '^' },
{ "help", no_argument, nullptr, 'h' },
{ nullptr, 0, nullptr, 0 }
};
Expand Down Expand Up @@ -459,14 +460,12 @@ int inject(int argc, char** argv)
}
break;

case 'q': app_desc.quiet = true; break;
case 'd': target_pid = atoi(optarg); break;
case 'q': app_desc.quiet = true; break;
case 'l': app_desc.log = false; break;
case '^': app_desc.detours = true; break;
case '_': ret = 0; is_autorun = true; break;

case 'l':
app_desc.log = false;
break;

case '?':
case 'h':
ret = 0;
Expand Down
2 changes: 2 additions & 0 deletions clink/app/src/loader/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ inline bool __streq(const WCHAR* a, const WCHAR* b)

//------------------------------------------------------------------------------
// This is exported so that `clink testbed --hook` can simulate injection.
// This MUST import all APIs that Clink hooks, so that the EXE has IAT entries
// for them.
extern "C" {
__declspec(dllexport) void __stdcall testbed_hook_loop()
{
Expand Down
6 changes: 6 additions & 0 deletions clink/app/src/utils/app_context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,12 @@ bool app_context::is_quiet() const
return m_desc.quiet;
}

//------------------------------------------------------------------------------
bool app_context::is_detours() const
{
return m_desc.detours;
}

//------------------------------------------------------------------------------
void app_context::get_binaries_dir(str_base& out) const
{
Expand Down
3 changes: 2 additions & 1 deletion clink/app/src/utils/app_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class app_context
bool log = true;
bool inherit_id = false; // Allow auto-detecting id from environment.
bool force = false; // Skip host check (for testbed).
char unused = 0;
bool detours = false; // Use Detours for hooking, instead of IAT.
char state_dir[510]; // = {}; (this crashes cl.exe v18.00.21005.1)
char script_path[510]; // = {}; (this crashes cl.exe v18.00.21005.1)
};
Expand All @@ -29,6 +29,7 @@ class app_context
int get_id() const;
bool is_logging_enabled() const;
bool is_quiet() const;
bool is_detours() const;
void get_binaries_dir(str_base& out) const;
void get_state_dir(str_base& out) const;
void get_autostart_command(str_base& out) const;
Expand Down
Loading

0 comments on commit ba71118

Please sign in to comment.