0

We are trying to understand some of the finer details of locating a symbol at runtime. I've already reviewed Jeffrey Richter's Programming Applications for Microsoft Windows, Chapters 19 (DLL Basics) and 20 (DLL Advanced). Richter is a bit more comprehensive and more cohesive than MSDN.

We are trying to combine implicit library linking with dynamic symbol location. We want to avoid calls to LoadLibrary because of some of the potential security hazards. We are also trying to avoid loader lock issues if a user wanders into doing too much in a DllMain function.

#include <windows.h>
#pragma comment(lib, "kernel32")

extern "C" {
  typedef BOOL (WINAPI * PFN_GOR)(HANDLE, LPOVERLAPPED, LPDWORD, BOOL);
  typedef BOOL (WINAPI * PFN_GORX)(HANDLE, LPOVERLAPPED, LPDWORD, DWORD, BOOL);
}
int main(int argc, char* argv[])
{
    HINSTANCE hInst = GetModuleHandle("kernel32");
    PFN_GOR pfn1 = hInst ? (PFN_GOR)GetProcAddress(hInst, "GetOverlappedResult") : NULL;
    PFN_GORX pfn2 = hInst ? (PFN_GORX)GetProcAddress(hInst, "GetOverlappedResultEx") : NULL;

    std::cout << "kernel32: " << std::hex << (void*)hInst << std::endl;
    std::cout << "GetOverlappedResult: " << std::hex << (void*)pfn1 << std::endl;
    std::cout << "GetOverlappedResultEx: " << std::hex << (void*)pfn2 << std::endl;

    return 0;
}

We kind of got lucky with GetOverlappedResult and GetOverlappedResultEx because kernel32.dll provides both of them regardless of the platform. kernel32.dll has another nice property because every Windows application links to it, and there's no way to disgorge it.

Random numbers from the platform appear to be a little more troublesome. On Windows 7 and below we have CryptGenRandom from advapi32.dll; while Windows 10 and above uses BCryptGenRandom from bcrypt.dll. Windows 8 is a grey area because some versions, like Windows Phone 8, do not offer anything. We believe we can guard inclusion of a library based on WINVER or _WINNT_VER.

I feel like the pattern of implicit library linking combined with GetModuleHandle and GetProcAdress is unusual but it meets our requirements. Its unusual because it uses implicit linking and GetModuleHandle rather than LoadLibrary. I also cannot find text that forbids implicit linking and GetModuleHandle.

It meets our requirements because we will not be responsible for insecure library loading due to binary planting and other DLL redirection tricks. We will also avoid DoS'es from accidental mis-use by doing too much in a DLLmain.

My question is, is the code a legal combination or pattern. If its a defective pattern, then what is the defect?


We support Windows XP and Visual Studio .Net through Windows 10/Phone 10/Store 10 using Visual Studio 2015.

On Windows XP the code produces the following result:

>cl.exe /TP /EHsc test.cxx /Fetest.exe
...
>.\test.exe
kernel32: 7D4C0000
GetOverlappedResult: 7D52E12C
GetOverlappedResultEx: 00000000

On Windows 7 the code produces the following result:

>cl.exe /TP /EHsc test.cxx /Fetest.exe
...
>.\test.exe
kernel32: 772A0000
GetOverlappedResult: 772CCC69
GetOverlappedResultEx: 00000000

On Windows 8 the code produces the following result (Windows 10 is similar):

>cl.exe /TP /EHsc test.cxx /Fetest.exe
...
>.\test.exe
kernel32: 74FD0000
GetOverlappedResult: 74FEF8C0
GetOverlappedResultEx: 7675C4D0

On Windows 8 and 10 we can only test the cross-compile and link with ARM Developer Prompt. We test the compile for Desktop, Phone and Store using the following additional CXXFLAGS:

  • To test Desktop app, add the following CXXFLAGS:
    • /DWINAPI_FAMILY=WINAPI_FAMILY_DESKTOP_APP
  • To test Windows Store app, add the following CXXFLAGS:
    • /DWINAPI_FAMILY=WINAPI_FAMILY_APP
  • To test Windows Phone, add the following CXXFLAGS:
    • /DWINAPI_FAMILY=WINAPI_FAMILY_PHONE_APP
  • To test Surface RT (ARM tablet), add the following CXXFLAGS:
    • /D_ARM_WINAPI_PARTITION_DESKTOP_SDK_AVAILABLE=1 /DWINAPI_FAMILY=WINAPI_FAMILY_DESKTOP_APP

I'm finding lots of hits like Implicit vs. Explicit linking to a DLL, but I have not been able to find one that examines some of the security issues that LoadLibrary can impose, and how to avoid LoadLibrary altogether.

Community
  • 1
  • 1
jww
  • 97,681
  • 90
  • 411
  • 885
  • For those who are interested, here is the issue comment we are evaluating and trying to tend to: [comment 233405697](http://github.com/weidai11/cryptopp/issues/178#issuecomment-233405697). – jww Oct 09 '16 at 08:52
  • After reading the github link it seems you are trying to get your min. and target platforms properly implemented. I would suggest another solution to this issue altogether: Use the [Linker Support for Delay-Loaded DLLs](https://msdn.microsoft.com/en-us/library/151kt790.aspx), specify the target version through preprocessor symbols, and gracefully deal with unavailable features by providing custom delay-load [helper functions](https://msdn.microsoft.com/en-us/library/09t6x5ds.aspx). – IInspectable Oct 09 '16 at 11:48
  • 1
    I imagine you're already aware of this, but just to spell it out for future readers, this approach won't work for Windows Phone and I don't think it will work for UWP either. If you're supporting those, you'll need to use conditional compilation - but it won't be conditional on `_WIN32_WINNT` which seems to be the case that is annoying you. – Harry Johnston Oct 10 '16 at 01:11
  • Also, note that in the specific case of GetOverlappedResult vs. GetOverlappedResultEx, you probably don't need to use GetProcAddress. AFAIK, GetOverlappedResult is only deprecated for Phone and UWP builds, not for desktop builds. So unless you're actually going to use the extra parameter, don't bother. (PS: at one point you wrote "Eventually it will break the library when GetOverlappedResult goes away." As far as desktop builds are concerned, that won't happen. Not until Windows itself reaches end-of-life, at any rate.) – Harry Johnston Oct 10 '16 at 01:16
  • @HarryJohnston - *"this approach won't work for Windows Phone and I don't think it will work for UWP either..."* - No, I was not aware. Please answer because that's one of the platforms we support. I'm especially interested in knowing what's wrong on the mobile platforms since the docs don't forbid it and our testing does not reveal a problem. In the big picture, we are trying to get away from all those stupid version macros and include files that have cause us so many problems, from dirty compiles due to warnings and then the subsequent bug reports and user discussions. – jww Oct 10 '16 at 01:39
  • OK. I should be getting back to work now, but I'll post something later. The upshot will be pretty much the same [as my previous answer](http://stackoverflow.com/a/37670348/886887) though ... it sounds to me as though the conceptual problem is that you're trying to treat Windows Phone/Store/UWP/Desktop as being basically the same platform, and they're not. You will need different code for Desktop builds and Phone builds for basically the same reason you need different code for Windows and Linux, or perhaps a better analogy would be Linux and BSD. YMMV. – Harry Johnston Oct 10 '16 at 01:43
  • ... I'm far from expert in this area, mind you. I only do Desktop. It might make more sense to post a new question or rephrase this one so as to clarify that your library needs to support Phone/Store/UWP (where you know what you're building for at build time) as well as desktop (where you don't). – Harry Johnston Oct 10 '16 at 01:46
  • @HarryJohnston - And for completeness, I build and test for WinRT (ARM tablets, `_ARM_WINAPI_PARTITION_DESKTOP_SDK_AVAILABLE=1 /DWINAPI_FAMILY=WINAPI_FAMILY_DESKTOP_APP`), Windows Phone (`WINAPI_FAMILY=WINAPI_FAMILY_PHONE_APP`) and Windows Store (`WINAPI_FAMILY=WINAPI_FAMILY_APP`). Its all Nmake based because we can't use MSBuild at the moment. MSbuild is undocumented and broken with respect to ARM and Store apps. – jww Oct 10 '16 at 01:46
  • *I'm especially interested in knowing what's wrong on the mobile platforms since the docs don't forbid it* ... if you look at the documentation for GetModuleHandle, under Requirements, you'll find it says "desktop apps only". Oddly enough GetProcAddress doesn't. I'm not sure whether that means there's some other way of getting the handle that will work in Phone/Store/UWP. – Harry Johnston Oct 10 '16 at 01:49
  • @HarryJohnston - *"[previous discussion] The underlying problem here is that you're using _WIN32_WINNT to determine which code path to take, but not actually setting it."* - OK, thanks. Let me get a new question together that builds on this one. – jww Oct 10 '16 at 02:01
  • *[quoting myself] I'm not sure whether that means there's some other way of getting the handle* ... yeah, I guess that's LoadPackagedLibrary, but unfortunately that's only available on Windows 8 so won't help you. Re the previous discussion, you probably *don't* need to set _WIN32_WINNT, because if you're building for Desktop you really can do the dynamic thing (or just stick with GetOverlappedResult if you don't need the extra parameter) but you do need to work out whether you're on Desktop or not. See "How to: Use Existing C++ Code in a Universal Windows Platform App" in my earlier answer. – Harry Johnston Oct 10 '16 at 02:06

1 Answers1

3

Not much to say here. It is perfectly legal to use GetProcAddress with a module handle obtained by calling GetModuleHandle.

Update: I wrote this based on the original form of the question which did not make it clear that the question is meant to cover mobile platforms as well as desktop.

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
  • Thanks David. That's kind of what I thought, but I wanted to make sure I was not missing something. Let me give other folks time for feedback. I'll accept if there are no defects raised. Thanks again. – jww Oct 09 '16 at 16:13
  • David, you might want to weigh in on the discussion in the comments, particularly if I'm wrong in thinking that this won't work in Store/Phone/UWP builds. (GetModuleHandle is documented as desktop-only, but from what the OP says it this code actually build without complaining. Perhaps it would fail validation though?) – Harry Johnston Oct 10 '16 at 02:28