9

I have some Visual C++ code that receives a pointer to a buffer with data that needs to be processed by my code and the length of that buffer. Due to a bug outside my control, sometimes this pointer comes into my code uninitialized or otherwise unsuitable for reading (i.e. it causes a crash when I try to access the data in the buffer.)

So, I need to verify this pointer before I use it. I don't want to use IsBadReadPtr or IsBadWritePtr because everyone agrees that they're buggy. (Google them for examples.) They're also not thread-safe -- that's probably not a concern in this case, though a thread-safe solution would be nice.

I've seen suggestions on the net of accomplishing this by using VirtualQuery, or by just doing a memcpy inside an exception handler. However, the code where this check needs to be done is time sensitive so I need the most efficient check possible that is also 100% effective. Any ideas would be appreciated.

Just to be clear: I know that the best practice would be to just read the bad pointer, let it cause an exception, then trace that back to the source and fix the actual problem. However, in this case the bad pointers are coming from Microsoft code that I don't have control over so I have to verify them.

Note also that I don't care if the data pointed at is valid. My code is looking for specific data patterns and will ignore the data if it doesn't find them. I'm just trying to prevent the crash that occurs when running memcpy on this data, and handling the exception at the point memcpy is attempted would require changing a dozen places in legacy code (but if I had something like IsBadReadPtr to call I would only have to change code in one place).

4
  • Can you post a callstack (all the way up to main or WinMain)?
    – MSN
    Jan 30, 2009 at 16:25
  • 1
    I added a paragraph to my answer: maybe you can use IsBadReadPtr safely, if you previously touch all your stack pages.
    – ChrisW
    Jan 30, 2009 at 17:41
  • I don't know, off-hand, what's wrong with your sample code using VirtualQuery. A generic problem is that even if VirtualQuery says the memory is good, another thread might release the memory after you test it but before you try to read it: so, given multi-threading, testing it in advance isn't ...
    – ChrisW
    Jan 30, 2009 at 20:40
  • ,,, a substitute for protecting your read-attempt within an exception-handler.
    – ChrisW
    Jan 30, 2009 at 20:41

12 Answers 12

19
bool IsBadReadPtr(void* p)
{
    MEMORY_BASIC_INFORMATION mbi = {0};
    if (::VirtualQuery(p, &mbi, sizeof(mbi)))
    {
        DWORD mask = (PAGE_READONLY|PAGE_READWRITE|PAGE_WRITECOPY|PAGE_EXECUTE_READ|PAGE_EXECUTE_READWRITE|PAGE_EXECUTE_WRITECOPY);
        bool b = !(mbi.Protect & mask);
        // check the page is not a guard page
        if (mbi.Protect & (PAGE_GUARD|PAGE_NOACCESS)) b = true;

        return b;
    }
    return true;
}
4
  • 3
    I personally found this code very useful and understandable.
    – Eugene K
    Jun 27, 2016 at 4:05
  • 2
    Explain your usage and understanding, don't just write comments. :P
    – Andrew
    Sep 24, 2016 at 7:25
  • it prints valid ptr is i will write (LPVOID) base + 0xFFFFFFF 0xFFFFFFF is invalid location from base address of process. .
    – HeavenHM
    Apr 3, 2018 at 22:30
  • @HaSeeBMiR That depends, in a 64-bit application this could be a valid address. I would also like to add that 0xFFFFFFF is not the 32-bit max. There's only 7 F's, 32-bit goes until 8 or 0xFFFFFFFF. Sep 10, 2018 at 12:49
10

a thread-safe solution would be nice

I'm guessing it's only IsBadWritePtr that isn't thread-safe.

just doing a memcpy inside an exception handler

This is effectively what IsBadReadPtr is doing ... and if you did it in your code, then your code would have the same bug as the IsBadReadPtr implementation: http://blogs.msdn.com/oldnewthing/archive/2006/09/27/773741.aspx

--Edit:--

The only problem with IsBadReadPtr that I've read about is that the bad pointer might be pointing to (and so you might accidentally touch) a stack's guard page. Perhaps you could avoid this problem (and therefore use IsBadReadPtr safely), by:

  • Know what threads are running in your process
  • Know where the threads' stacks are, and how big they are
  • Walk down each stack, delberately touching each page of the stack at least once, before you begin to call isBadReadPtr

Also, the some of the comments associated with the URL above also suggest using VirtualQuery.

3
  • Thanks. I think the VirtualQuery idea is the most promising, but I don't really understand it. Do you have any idea why the code above doesn't work reliably?
    – jeffm
    Jan 30, 2009 at 17:56
  • 1
    This wouldn't work in general. When you touch the stack guard page it will grow the stack only if you touched it from the thread that owns it! Jul 4, 2010 at 9:05
  • 1
    One problem is if you get a bogus pointer that points to real memory, it may be that while the first few reads from that area of memory are ok a few bytes down (where it crosses a page boundary) may be unallocated and invalid memory.
    – James
    Mar 3, 2012 at 13:30
6

The reason these functions are bad to use is that the problem can't be solved reliably.

What if the function you're calling returns a pointer to memory that is allocated, so it looks valid, but it's pointing to other, unrelated data, and will corrupt your application if you use it.

Most likely, the function you're calling actually behaves correctly, and you are misusing it. (Not guaranteed, but that is often the case.)

Which function is it?

3
  • It's actually probably not a problem if the data can be read from but is bogus, because my code is looking for some specific data patterns and if they aren't there it will just ignore the data. It's the random crashes I'm trying to fix. :-)
    – jeffm
    Jan 30, 2009 at 16:36
  • But it still means that the function you called has ventured into undefined territory. You can't really rely on it to not corrupt your program state any more. But of course the if you're willing to put up with that, simply catch the acccess violation and pretend nothing happened.
    – jalf
    Jan 30, 2009 at 17:27
  • There's also the risk that these bit patterns are found in the random data it pointed to, which means you'll be the one corrupting your program state ;)
    – jalf
    Jan 30, 2009 at 17:27
2

The fastest solution I can think of is to consult the virtual memory manager using VirtualQuery to see if there is a readable page at the given address, and cache the results (however any caching will reduce the accuracy of the check).

Example (without caching):

BOOL CanRead(LPVOID p)
{
  MEMORY_BASIC_INFORMATION mbi;
  mbi.Protect = 0;
  ::VirtualQuery(((LPCSTR)p) + len - 1, &mbi, sizeof(mbi));
  return ((mbi.Protect & 0xE6) != 0 && (mbi.Protect & PAGE_GUARD) == 0);
}
1

Why can't you call the api

AfxIsValidAddress((p), sizeof(type), FALSE));

1
1

If the variable is uninitialized you are hosed. Sooner or later it's going to be an address for something you don't want to play with (like your own stack).

If you think you need this, and (uintptr_t)var < 65536 does not suffice (Windows does not allow allocating the bottom 64k), there is no real solution. VirtualQuery, etc. appear to "work" but sooner or later will burn you.

0

I am afraid you are out of luck - there is no way to reliably check the validity of a pointer. What Microsoft code is giving you bad pointers?

2
  • It's the Winsock 2 layered service provider (LSP) sample code.
    – jeffm
    Jan 30, 2009 at 16:18
  • If you're working on an LSP, get ready for many days of hair pulling. And get a good Dev Support contract from Microsoft. Jan 30, 2009 at 16:44
0

Any implementation of checking the validity of memory is subject to the same constriants that make IsBadReadPtr fail. Can you post an example callstack of where you want to check the validity of memory of a pointer passed to you from Windows? That might help other people (including me) diagnose why you need to do this in the first place.

0

If you have to resort to checking patterns in data, here are a few tips:

  • If you mention using IsBadReadPtr, you are probably developing for Windows x86 or x64.

  • You may be able to range check the pointer. Pointers to objects will be word aligned. In 32-bit windows, user-space pointers are in the range of 0x00401000-0x7FFFFFFF, or for large-address-aware applications, 0x00401000-0xBFFFFFFF instead (edit: 0x00401000-0xFFFF0000 for a 32-bit program on 64-bit windows). The upper 2GB/1GB is reserved for kernel-space pointers.

  • The object itself will live in Read/Write memory which is not executable. It may live in the heap, or it may be a global variable. If it is a global variable, you can validate that it lives in the correct module.

  • If your object has a VTable, and you are not using other classes, compare its VTable pointer with another VTable pointer from a known good object.

  • Range check the variables to see if they are possibly valid. For example, bools can only be 1 or 0, so if you see one with a value of 242, that's obviously wrong. Pointers can also be range checked and checked for alignment as well.

  • If there are objects contained within, check their VTables and data as well.

  • If there are pointers to other objects, you can check that the object lives in memory that is Read/Write and not executable, check the VTable if applicable, and range check the data as well.

If you do not have a good object with a known VTable address, you can use these rules to check if a VTable is valid:

  • While the object lives in Read/Write memory, and the VTable pointer is part of the object, the VTable itself will live in memory that is Read Only and not executable, and will be aligned to a word boundary. It will also belong to the module.
  • The entries of the VTable are pointers to code, which will be Read Only and Executable, and not writable. There is no alignment restrictions for code addresses. Code will belong to the module.
0

Here is what I use this just replaces the official microsoft ones by using #define's this way you can use the microsoft ones and not worry about them failing you.

// Check memory address access
const DWORD dwForbiddenArea = PAGE_GUARD | PAGE_NOACCESS;
const DWORD dwReadRights = PAGE_READONLY | PAGE_READWRITE | PAGE_WRITECOPY | PAGE_EXECUTE_READ | PAGE_EXECUTE_READWRITE | PAGE_EXECUTE_WRITECOPY;
const DWORD dwWriteRights = PAGE_READWRITE | PAGE_WRITECOPY | PAGE_EXECUTE_READWRITE | PAGE_EXECUTE_WRITECOPY;

template<DWORD dwAccessRights>
bool CheckAccess(void* pAddress, size_t nSize)
{
    if (!pAddress || !nSize)
    {
        return false;
    }

    MEMORY_BASIC_INFORMATION sMBI;
    bool bRet = false;

    UINT_PTR pCurrentAddress = UINT_PTR(pAddress);
    UINT_PTR pEndAdress = pCurrentAddress + (nSize - 1);

    do
    {
        ZeroMemory(&sMBI, sizeof(sMBI));
        VirtualQuery(LPCVOID(pCurrentAddress), &sMBI, sizeof(sMBI));

        bRet = (sMBI.State & MEM_COMMIT) // memory allocated and
            && !(sMBI.Protect & dwForbiddenArea) // access to page allowed and
            && (sMBI.Protect & dwAccessRights); // the required rights

        pCurrentAddress = (UINT_PTR(sMBI.BaseAddress) + sMBI.RegionSize);
    } while (bRet && pCurrentAddress <= pEndAdress);

    return bRet;
}

#define IsBadWritePtr(p,n) (!CheckAccess<dwWriteRights>(p,n))
#define IsBadReadPtr(p,n) (!CheckAccess<dwReadRights>(p,n))
#define IsBadStringPtrW(p,n) (!CheckAccess<dwReadRights>(p,n*2))

This approach is based on my understanding of Raymond Chen's blog post, If I'm not supposed to call IsBadXxxPtr, how can I check if a pointer is bad?

0
0

This is an old question but this part:

the code where this check needs to be done is time sensitive so I need the most efficient check possible that is also 100% effective

VirtualQuery() takes a kernel call, so the simple memcpy() in an exception handler will be faster for the case where the memory is okay to read most of the time.

__try
{
  memcpy(dest, src, size);
}__except(1){}

All stays in user mode when there is no exception. Maybe a bit slower for the use case where the memory is bad to read more than it is good (since it fires off a exception which is a round trip through the kernel and back).

You could also extend it with a custom memcpy loop and *size so you could return exactly how many bytes were actually read.

-1

if you are using VC++ then I suggest to use microsoft specific keywords __try __except to and catch HW exceptions

1
  • still crashes inside a __try \ __except :(
    – SSpoke
    Dec 19, 2020 at 4:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.