VerQueryValueA writes invalid memory outside of the resource block

Question

I am retrieving a version string for the current executable using the following code:

// http://stackoverflow.com/questions/13941837/how-to-get-version-info-from-resources
std::string get_version_string()
{
    auto hInst = GetModuleHandle(NULL);

// The following functions allocate persistent one-time space in the process's memory - they don't need to have results freed
    auto hResInfo = FindResourceA(hInst, MAKEINTRESOURCE(1), RT_VERSION);
    auto dwSize = SizeofResource(hInst, hResInfo);
    auto hResData = LoadResource(hInst, hResInfo);
    char *pRes = static_cast<char *>(LockResource(hResData));
    if ( !dwSize || !pRes ) return {};

// Copy is required because VerQueryValue modifies the object, but LoadResource's resource is non-modifiable.
// SizeofResource yielded the size in bytes, according to its documentation.
    std::vector<char> ResCopy(dwSize);
    std::copy(pRes, pRes + dwSize, ResCopy.begin());

// https://stackoverflow.com/a/1174697/1505939
    LPVOID pvFileVersion{};
    UINT iFileVersionLen{};

    if ( !VerQueryValueA(&ResCopy[0], "\\StringFileInfo\\040904E4\\FileVersion", &pvFileVersion, &iFileVersionLen) )
        return "(unknown)"s;

    char buf[200];
    sprintf(buf, "%p\n%p\n%p", &ResCopy[0], &ResCopy[0] + ResCopy.size(), pvFileVersion);
    debug_output ( buf );

    auto s = static_cast<char *>(pvFileVersion);
    return std::string( s, s + iFileVersionLen );
}

The VerQueryValue documentation, and other SO questions on the topic, indicate that VerQueryValue is supposed to return a pointer into the resource block. However, the output I get is:

000000000594b460
000000000594b748
000000000594b802

Furthermore, if I change the allocation of ResCopy to std::vector<char> ResCopy(dwSize + 0x200); , then I get the output:

000000000594b460
000000000594b948
000000000594b802

and the only thing I can conclude from this is that the VerQueryValueA function is doing an out-of-bounds write in the original case; it's writing past the end of the resource's size as was given by SizeofResource; it's writing outside of my vector.

Even though the function seems to work properly I suspect this might actually be a bug.

My question is: am I doing something wrong, or is this a bug in VerQueryValueA ? And how should I fix the problem?

Note: If I use VerQueryValueW then it does return a pointer inside ResCopy in the first place.

This answer seems to allude to the issue however I'm not using GetFileVersionInfo (which requires a filename, there doesn't seem to be any equivalent function that takes the module handle).

The greater purpose of this is to be able to log my application's version string in the log file, and trying to find and open a file based on filename seems like a bunch more possible points of failure when we have obviously already loaded the executable to be running it.

I rolled back the edit changing "writes" to "reads". The `VerQueryValueA` function must cause a write to `594b802` , otherwise the function would not appear to be working (it does run successfully and create a `std::string` holding the correct value). There certainly was not that value at 594b802 prior to the call to `VerQueryValueA`. — M.M, Feb 02 '18 at 22:56

Remy Lebeau · Accepted Answer · 2018-02-02T07:27:47.257

GetFileVersionInfo() performs fixups and data conversions that VerQueryValue() relies on. Raymond Chen even wrote a blog article about it:

The first parameter to VerQueryValue really must be a buffer you obtained from GetFileVersionInfo

The documentation for the VerQueryValue function states that the first parameter is a "pointer to the buffer containing the version-information resource returned by the GetFileVersionInfo function." Some people, however, decide to bypass this step and pass a pointer to data that was obtained some other way, and then wonder why VerQueryValue doesn't work.

The documentation says that the first parameter to VerQueryValue must be a buffer returned by the GetFileVersionInfo function for a reason. The buffer returned by GetFileVersionInfo is an opaque data block specifically formatted so that VerQueryValue will work. You're not supposed to look inside that buffer, and you certainly can't try to "obtain the data some other way". Because if you do, VerQueryValue will look for something in a buffer that is not formatted in the manner the function expects.

Other than querying the VS_FIXEDFILEINFO at the very beginning of the resource data, it is really not safe to use VerQueryValue() to query other version data from the raw resource data. That data hasn't been prepared for VerQueryValue() to use. The answers to the question you linked to even state this, as does the above article:

If it wasn't obvious enough from the documentation that you can't just pass a pointer to a version resource obtained "some other way", it's even more obvious once you see the format of 32-bit version resources. Notice that all strings are stored in Unicode. But if you call the ANSI version VerQueryValueA to request a string, the function has to give you a pointer to an ANSI string. There is no ANSI version of the string in the raw version resource, so what can it possibly return? You can't return a pointer to something that doesn't exist. VerQueryValueA needs to produce an ANSI string, and it does so from memory that GetFileVersionInfo prepared when the resources were extracted.

For what you are attempting to do, querying the VS_FIXEDFILEINFO from the copied resource is all you need. It contains the version number you are looking for, is language agnostic, and is not dependent on GetFileVersionInfo():

std::string get_version_string()
{
    auto hInst = GetModuleHandle(NULL);
    auto hResInfo = FindResourceA(hInst, MAKEINTRESOURCE(1), RT_VERSION);
    if ( !hResInfo ) return {};
    auto dwSize = SizeofResource(hInst, hResInfo);
    if ( !dwSize ) return {};
    auto hResData = LoadResource(hInst, hResInfo);
    char *pRes = static_cast<char *>(LockResource(hResData));
    if ( !pRes ) return {};

    std::vector<char> ResCopy(pRes, pRes + dwSize);

    VS_FIXEDFILEINFO *pvFileInfo;
    UINT uiFileInfoLen;

    if ( !VerQueryValueA(ResCopy.data(), "\\", reinterpret_cast<void**>(&pvFileInfo), &uiFileInfoLen) )
        return "(unknown)"s;

    char buf[25];
    int len = sprintf(buf, "%hu.%hu.%hu.%hu", 
        HIWORD(pvFileInfo->dwFileVersionMS),
        LOWORD(pvFileInfo->dwFileVersionMS),
        HIWORD(pvFileInfo->dwFileVersionLS),
        LOWORD(pvFileInfo->dwFileVersionLS)
    );

    return std::string(buf, len);
}

Even if it works for `VS_FIXEDFILEINFO`, it is still undocumented to call `VerQueryValue` like that, so the reliability of this method is questionable. — zett42, Feb 02 '18 at 13:08
Your code retrieves different information. The value in `StringFileInfo\\...\\FileVersion` is freeform text , e.g. `1.0.0-alpha`; I would like to retrieve specifically that value. Also (if I understand the rest of your answer correctly) even this code still relies on undocumented behaviour -- it could happen that in a future update to Windows, even this use of VerQueryValueA would work out of bounds of the copied block, or fail in some other way — M.M, Feb 02 '18 at 22:46
@M.M: "*The value in `StringFileInfo\\...\\FileVersion` is freeform text , e.g. `1.0.0-alpha`*" - then you need to switch to `VerQueryValueW()`, or use `GetFileVersionInfo()` to use `VerQueryValueA()`. "*Also ... even this code still relies on undocumented behaviour*" - technically, yes. — Remy Lebeau, Feb 02 '18 at 22:50
@M.M: "*it could happen that in a future update to Windows, even this use of VerQueryValueA would work out of bounds of the copied block, or fail in some other way*" - not likely, given the [structure of a `VERSIONINFO` resource](https://msdn.microsoft.com/en-us/library/windows/desktop/aa381058.aspx) and [32bit version resources](https://blogs.msdn.microsoft.com/oldnewthing/20061221-02/?p=28643), and [how `VerQueryValue()` returns pointers to resource data](https://blogs.msdn.microsoft.com/oldnewthing/20061226-06/?p=28603). There will still be a `VS_FIXEDFILEINFO` struct for it to point at. — Remy Lebeau, Feb 02 '18 at 22:57
VerQueryValue can clearly write new things into the prepared resource data and return pointer to that, as shown by my original code — M.M, Feb 02 '18 at 22:58
@M.M: [The evolution of version resources – corrupted 32-bit version resources](https://blogs.msdn.microsoft.com/oldnewthing/20061222-00/?p=28623): "*As a result of this mess, the VerQueryValue function keeps its eyes open and anticipates that the version resource it was given to parse may have been generated by one of these buggy version resource compilers **and goes to some extra effort to accommodate those bugs**.*" - so, it is possible that `VerQueryValue()` may have to write data back into the memory buffer to fix things it thinks are wrong. — Remy Lebeau, Feb 02 '18 at 23:02

score 1 · Answer 2 · answered Feb 02 '18 at 08:19

1

As Remy Lebeau already explained, you are using the API in an undocumented way.

I'm not using GetFileVersionInfo (which requires a filename, there doesn't seem to be any equivalent function that takes a module handle).

The solution is simple: Call GetModuleFileName() to retrieve the file path of your executable, then call GetFileVersionInfoSize(), GetFileVersionInfo(), VerQueryValue() as documented.

answered Feb 02 '18 at 08:19

zett42

25,437
3
35
72

GetFileVersionInfo will require doing filesystem opening the file, I'm worried that this may have a chance of failure, e.g. if it's a network drive and the network is congested or intermittently fails (could this even block waiting for network?) – M.M Feb 02 '18 at 22:35
@M.M When you are using the low level resource APIs like `FindResource()`, `LoadResource()`, there will be file access aswell because at some point the memory-mapped file *must* be read. – zett42 Feb 03 '18 at 00:18
OK, I was figuring that that stuff would have been loaded into memory when the executable was loaded – M.M Feb 03 '18 at 00:30
@M.M When the executable is loaded, it is just mapped into memory (as if you would call `CreateFileMapping()`, but a little more complicated). Only when the pages are actually read, will they be physically loaded from the file (in your code when `std::copy(pRes,...)` is called. – zett42 Feb 03 '18 at 10:32

VerQueryValueA writes invalid memory outside of the resource block

2 Answers2

Linked