Fix context setting errors in DetourEnumerateImports API (#266)

* fix context setting errors in DetourEnumerateImports API

* Fix SAL annotation on DetouerEnumerateImportThunk 

SAL annotation for VoidContext should just be In rather than In_opt (this is consistent with how the existing DetouerEnumerateImportThunk is structured).  

This was causing an error in the static analysis checks in the CI build.

* Update test_module_api.cpp

Comparison of BOOL vs. bool, this should fix it.

---------

Co-authored-by: g00356712 <[email protected]>
Co-authored-by: Jeff Bisset <[email protected]>

Author: 3 people

src/modules.cpp

@@ -645,6 +645,7 @@ BOOL WINAPI DetourEnumerateImportsEx(_In_opt_ HMODULE hModule,
 struct _DETOUR_ENUMERATE_IMPORTS_THUNK_CONTEXT
 {
     PVOID pContext;
+    PF_DETOUR_IMPORT_FILE_CALLBACK pfImportFile;
     PF_DETOUR_IMPORT_FUNC_CALLBACK pfImportFunc;
 };
 
@@ -664,6 +665,19 @@ DetourEnumerateImportsThunk(_In_ PVOID VoidContext,
     return pContext->pfImportFunc(pContext->pContext, nOrdinal, pszFunc, ppvFunc ? *ppvFunc : NULL);
 }
 
+static
+BOOL
+CALLBACK
+DetourEnumerateImportsFile(_In_ PVOID VoidContext,
+                           _In_opt_ HMODULE hModule,
+                           _In_opt_ LPCSTR pszFile)
+{
+    _DETOUR_ENUMERATE_IMPORTS_THUNK_CONTEXT const * const
+        pContext = (_DETOUR_ENUMERATE_IMPORTS_THUNK_CONTEXT*)VoidContext;
+    return pContext->pfImportFile(pContext->pContext, hModule, pszFile);
+}
+
+
 BOOL WINAPI DetourEnumerateImports(_In_opt_ HMODULE hModule,
                                    _In_opt_ PVOID pContext,
                                    _In_opt_ PF_DETOUR_IMPORT_FILE_CALLBACK pfImportFile,
@@ -674,11 +688,10 @@ BOOL WINAPI DetourEnumerateImports(_In_opt_ HMODULE hModule,
         return FALSE;
     }
 
-    _DETOUR_ENUMERATE_IMPORTS_THUNK_CONTEXT const context = { pContext, pfImportFunc };
-
+    _DETOUR_ENUMERATE_IMPORTS_THUNK_CONTEXT const context = { pContext, pfImportFile, pfImportFunc };
     return DetourEnumerateImportsEx(hModule,
                                     (PVOID)&context,
-                                    pfImportFile,
+                                    &DetourEnumerateImportsFile,
                                     &DetourEnumerateImportsThunk);
 }
 

tests/test_module_api.cpp

@@ -506,7 +506,9 @@ BOOL WINAPI ImportFileCallback(PVOID pContext, HMODULE, PCSTR pszFile)
         reinterpret_cast<EnumerateImportsTestContext*>(pContext);
 
     context->ImportCount++;
-    context->ImportModuleFound |= Catch::contains(pszFile, "ntdll");
+    if (pszFile != NULL) {
+        context->ImportModuleFound |= Catch::contains(pszFile, "ntdll");
+    }
 
     return TRUE;
 }
@@ -568,6 +570,20 @@ TEST_CASE("DetourEnumerateImports", "[module]")
         REQUIRE( context.ImportFuncCount == 0 );
         REQUIRE_FALSE( context.ImportFuncFound );
     }
+
+    SECTION("The context transferred during the input parameter is the same as the context parsed in the callback.")
+    {
+        SetLastError(ERROR_INVALID_HANDLE);
+
+        EnumerateImportsTestContext context {};
+        auto success = DetourEnumerateImports(NULL, &context, ImportFileCallback, ImportFuncCallback);
+
+        REQUIRE( GetLastError() == 0 );
+        REQUIRE( success );
+
+        REQUIRE( context.ImportFuncCount != 0 );
+        REQUIRE( context.ImportCount != 0 );
+    }
 }
 
 TEST_CASE("DetourGetSizeOfPayloads", "[module]")