Tomas Berger
Tomas Berger

Reputation: 145

VirtualQueryEx unable to read memory, so can't dereference pointer (LPCVOID/uint8_t*)

I have a function to read the memory of an application

template<class T>
std::unordered_map<uint8_t, T> find_byte(T find) {
    std::cout << "Searching for value\n";

    std::unordered_map<uint8_t, T> mapping;

    // Turn the T bytes to a vector to use nice C++ functions
    std::vector<uint8_t> byte_check;
    if constexpr (std::is_same_v<T, std::string>) {
        byte_check = std::vector<uint8_t>(find.begin(), find.end());
    }
    else {
        uint8_t* data = static_cast<uint8_t*>(static_cast<void*>(&find));
        byte_check    = std::vector<uint8_t>(data, data + sizeof(find));
    }

    MEMORY_BASIC_INFORMATION info;
    for (uint8_t* addr = nullptr; VirtualQueryEx(m_proc, addr, &info, sizeof(info)) == sizeof(info); addr += info.RegionSize) {
        if (info.State == MEM_COMMIT && (info.Type == MEM_MAPPED || info.Type == MEM_PRIVATE)) {
            size_t read{};
            std::vector<uint8_t> mem_chunk;
            mem_chunk.resize(info.RegionSize);
            if (ReadProcessMemory(m_proc, addr, mem_chunk.data(), mem_chunk.size(), &read)) {
                mem_chunk.resize(read);
                for (auto pos = mem_chunk.begin();
                     mem_chunk.end() != (pos = std::search(pos, mem_chunk.end(), byte_check.begin(), byte_check.end()));
                     pos++) {
                    uint8_t* int_addr_ptr  = (addr + (pos - mem_chunk.begin()));
                    mapping[*int_addr_ptr] = find;
                }
            }
        }
    }
    return mapping;
}

It compiles just fine, however, it crashes it tries to dereference the int_addr_ptr pointer. After stepping through with a debugger, I noticed that the addr returned from VirtualQueryEx was unable to be read.

I assume the issue lies in how I dereference, but I don't know how to fix it. I have tired:

auto lpcvoid       = (addr + (pos - mem_chunk.begin()));
auto int_addr_ptr = reinterpret_cast<const uint8_t*>(lpcvoid);

from here, but it yielded no results.

I want to note that if I return a map of <uint8_t, T> it works fine, but wanted to avoid the pointers

Upvotes: 0

Views: 272

Answers (1)

Remy Lebeau
Remy Lebeau

Reputation: 597051

When you query a memory region, you should start scanning through its content from its BaseAddress, not from the address that you actually queried. VirtualQueryEx() may have to round down the address. info.RegionSize is relative to info.BaseAddress, so when calling ReadProcessMemory(), mem_chunk.size() will be too many bytes to read if addr > info.BaseAddress, you would need to subtract that offset from the size being read.

In the std::search() loop, pos should be incremented by the size of find that is being searched for, not by 1 byte. And addr should again be info.BaseAddress.

But more importantly, why is int_addr_ptr being dereferenced at all? Its value represents a memory address in another process, not in your own process. So, you can't simply dereference it to read a value, doing so will try to read the value from your process. If you want to read the value stored at that address, you will have to use ReadProcessMemory() instead. But, what is mapping supposed to be keying off of exactly? Why are you dereferencing int_addr_ptr to read a byte for a mapping key? Shouldn't you be keying the actual addresses where each copy of find is found? And what is the point of storing copies of find in the mapping?

A simpler vector<void*> would make more sense, since the caller already knows the find value it is searching for, so no need to repeat it in the output.

As an added optimization, I would suggest getting rid of the byte_check vector altogether, as it is an unnecessary memory allocation. You are using it only to have iterators for use with std::search(). Raw pointers are also valid iterators.

With that said, try something more like this:

template<class T>
std::vector<void*> find_value(const T &find) {
    std::cout << "Searching for value\n";

    std::vector<void*> found;
    std::vector<uint8_t> mem_chunk;

    const uint8_t *find_begin, *find_end;
    size_t find_size;

    if constexpr (std::is_same_v<T, std::string>) {
        find_begin = reinterpret_cast<const uint8_t*>(find.c_str());
        find_size = find.size();
    }
    else {
        find_begin = reinterpret_cast<const uint8_t*>(&find);
        find_size = sizeof(find);
    }

    find_end = find_begin + find_size;

    uint8_t *addr = reinterpret_cast<uint8_t*>(0), *base;
    MEMORY_BASIC_INFORMATION info;

    while (VirtualQueryEx(m_proc, addr, &info, sizeof(info))) {
        base = static_cast<uint8_t*>(info.BaseAddress);

        if (info.State == MEM_COMMIT && (info.Type == MEM_MAPPED || info.Type == MEM_PRIVATE)) {
            mem_chunk.reserve(info.RegionSize);

            size_t read;
            if (ReadProcessMemory(m_proc, info.BaseAddress, mem_chunk.data(), info.RegionSize, &read)) {
                auto start = mem_chunk.data(),
                     end = start + read,
                     pos = start;

                if (addr > base) {
                    pos += (addr - base);
                }

                while ((pos = std::search(pos, end, find_begin, find_end)) != end) {
                    found.push_back(base + (pos - start));
                    pos += find_size;
                }
            }
        }

        addr = base + info.RegionSize;
    }

    return found;
}

Upvotes: 1

Related Questions