Skip to content

Commit

Permalink
Fix a humongous bug in cuckoomon -- amazed not a single person spotte…
Browse files Browse the repository at this point in the history
…d this in years. Basically, any API resolved at runtime would simply not be hooked at all because of a string matching failure -- the wcsnicmp() in the code was using names from LdrLoadDll that had .dll appended to them, the length of that name, and trying to force a comparison against dll name strings without .dll appended -- it simply would never match. So instead you'd end up with loads of Native API hooks triggering because those were the only ones it was able to put in place.

Reported upstream.
  • Loading branch information
brad-sp committed Nov 10, 2014
1 parent fa160df commit 05060e8
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 5 deletions.
6 changes: 3 additions & 3 deletions cuckoomon.c
Original file line number Diff line number Diff line change
Expand Up @@ -387,11 +387,11 @@ static hook_t g_hooks[] = {
// error testing with hook_jmp_direct only
#define HOOKTYPE HOOK_JMP_DIRECT

void set_hooks_dll(const wchar_t *library, int len)
void set_hooks_dll(const wchar_t *library)
{
for (int i = 0; i < ARRAYSIZE(g_hooks); i++) {
if(!wcsnicmp(g_hooks[i].library, library, len)) {
hook_api(&g_hooks[i], HOOKTYPE);
if(!wcsicmp(g_hooks[i].library, library)) {
hook_api(&g_hooks[i], HOOKTYPE);
}
}
}
Expand Down
9 changes: 7 additions & 2 deletions hook_special.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
#include "hook_sleep.h"
#include "misc.h"

void set_hooks_dll(const wchar_t *library, int len);
void set_hooks_dll(const wchar_t *library);

HOOKDEF2(NTSTATUS, WINAPI, LdrLoadDll,
__in_opt PWCHAR PathToFile,
Expand Down Expand Up @@ -61,7 +61,12 @@ HOOKDEF2(NTSTATUS, WINAPI, LdrLoadDll,
if(NT_SUCCESS(ret)) {
// unoptimized, but easy
add_all_dlls_to_dll_ranges();
set_hooks_dll(library.Buffer, library.Length >> 1);
// we ensure null termination via the COPY_UNICODE_STRING macro above, so we don't need a length
// first strip off the .dll
PWCHAR end = wcsrchr(library.Buffer, L'.');
if (end && !wcsicmp(end, L".dll"))
*end = L'\0';
set_hooks_dll(library.Buffer);
}

return ret;
Expand Down

0 comments on commit 05060e8

Please sign in to comment.