Click here to Skip to main content
15,880,469 members
Articles / Desktop Programming / MFC

The bug in MFC tool tip control

Rate me:
Please Sign up or sign in to vote.
4.78/5 (16 votes)
13 Sep 2010CPOL2 min read 38K   13   14
The bug in MFC tool tip control

Yesterday, I encountered the bug in MFC’s tool tip control implementation.
The bug dozes in the following code:

C++
LRESULT CToolTipCtrl::OnAddTool(WPARAM wParam, LPARAM lParam)
{
    TOOLINFO ti = *(LPTOOLINFO)lParam;       <---------------------------- HERE
    if ((ti.hinst == NULL) && (ti.lpszText != LPSTR_TEXTCALLBACK)
        && (ti.lpszText != NULL))
    {
        void* pv;
        if (!m_mapString.Lookup(ti.lpszText, pv))
            m_mapString.SetAt(ti.lpszText, NULL);
        // set lpszText to point to the permanent memory associated
        // with the CString
        VERIFY(m_mapString.LookupKey(ti.lpszText, ti.lpszText));
    }
    return DefWindowProc(TTM_ADDTOOL, wParam, (LPARAM)&ti);
}

If you look at the definition of TOOLINFO structure closely, you’ll find that this structure has a variable size which depends on the compilation macros defined:

C++
typedef struct {
  UINT      cbSize;
  UINT      uFlags;
  HWND      hwnd;
  UINT_PTR  uId;
  RECT      rect;
  HINSTANCE hinst;
  LPTSTR    lpszText;
#if (_WIN32_IE >= 0x0300)
  LPARAM    lParam;
#endif 
#if (_WIN32_WINNT >= Ox0501)
  void      *lpReserved;
#endif 
} TOOLINFO, *PTOOLINFO, *LPTOOLINFO; 

That is, a structure pointed to by lParam can have a size from 40 to 48 bytes long depending on the compilation macros defined and can’t be just copied to structure created inside the MFC module. Dealing with variable-size structures, you can copy only a common part of a structure or analyze its version first (that is the cbSize field was made for!).

We can easily figure out what TOOLINFO’s size can be expected by MFC:

ASM
mfc71!CToolTipCtrl::OnAddTool:
7c1a21e5 55              push    ebp
7c1a21e6 8bec            mov     ebp,esp
7c1a21e8 83ec30          sub     esp,30h
7c1a21eb 53              push    ebx
7c1a21ec 56              push    esi
7c1a21ed 8b750c          mov     esi,dword ptr [ebp+0Ch]
7c1a21f0 57              push    edi
7c1a21f1 8bd9            mov     ebx,ecx
7c1a21f3 6a0c            push    0Ch
7c1a21f5 59              pop     ecx
7c1a21f6 8d7dd0          lea     edi,[ebp-30h]
7c1a21f9 f3a5            rep movs dword ptr es:[edi],dword ptr [esi]  

The size of TOOLINFO inside the MFC module is equal to 0xc*4 = 0x30 = 48 bytes. And the size of TOOLINFO structure inside our product’s module was equal to 44 bytes!

Of course, in most cases nothing criminal will happen. CToolTipCtrl::OnAddTool just copies 4 extra bytes – it's eventually not even used. The only case when the problem may arise – if those 4 extra bytes don’t have read access. Though this situation is hardly possible if your TOOLINFO structure resides on the stack, what about heap memory? With probability far from zero, your structure can occupy last free bytes in a memory page and next page can be not committed yet. This is exactly the case which I encountered. As a result – access violation.

Note that the problem can appear only if you allocate memory for TOOLINFO structure inside your own module, that is, use SendMessage approach.

I found the bug in MFC 7.1 library, but it’s not fixed even in MFC 10.0.

The Ways to Workaround

There are several obvious ways to workaround the problem:

  1. Do not use SendMessage approach to register a tool with a tool tip control. Use CToolTipCtrl::AddTool method instead.

    In case you cannot avoid SendMessage approach (for static controls, for example), you can do the following:

  2. Compile your module which uses tool tip control with all macros available (_WIN32_IE and _WIN32_WINNT).
  3. Reserve extra bytes (4 or 8, depending on your options) which can be copied without impact.

    For example like this:

    C++
    // Extend TOOLINFO on 8 bytes
    struct TOOLINFO_EX : public TOOLINFO
    {
      DWORD m_dwReserve1;
      DWORD m_dwReserve2;
    };
    
    // Allocate memory from heap (remember, it’s just example :))
    TOOLINFO* pti = new TOOLINFO_EX;
    
    // Initialization
    
    // Register a tool
    toolTipCtrl.SendMessage(TTM_ADDTOOL, 0, (LPARAM) pti); 

License

This article, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)


Written By
Software Developer (Senior) Transas Technologies
Russian Federation Russian Federation
This member has not yet provided a Biography. Assume it's interesting and varied, and probably something to do with programming.

Comments and Discussions

 
GeneralHarmless IMHO Pin
Martin Richter [rMVP C++]23-Sep-10 20:43
Martin Richter [rMVP C++]23-Sep-10 20:43 
As long as the MFC Version you are talking about are all compiled with _WIN32_WINNT >= Ox0501 there should be no problem at all. At least it isn't a problem for VC10. But AFAIK VC 7.1 is also compiled with awareness of Windows XP!

So there is room enough to take a full copy of the data and it can be stored on the stack without any further problem. Reading more data from the stack might be a problem if the data is allocated on the heap and on a boundary.

But also this is only a problem if you have settings to compile for W2K as a target. But again: VC10 doen't target W2K any longer. It is not a bug here!
--
Martin Richter (MVP for C++) WWJD http://blog.m-ri.de
"A well-written program is its own heaven; a poorly written program is its own hell!" The Tao of Programming

GeneralRe: Harmless IMHO Pin
Andrey Permamedov23-Sep-10 22:00
Andrey Permamedov23-Sep-10 22:00 
GeneralRe: Harmless IMHO Pin
Martin Richter [rMVP C++]23-Sep-10 22:51
Martin Richter [rMVP C++]23-Sep-10 22:51 
GeneralRe: Harmless IMHO Pin
Andrey Permamedov23-Sep-10 23:24
Andrey Permamedov23-Sep-10 23:24 
GeneralRe: Harmless IMHO [modified] Pin
Martin Richter [rMVP C++]23-Sep-10 23:41
Martin Richter [rMVP C++]23-Sep-10 23:41 
GeneralRe: Harmless IMHO Pin
Andrey Permamedov24-Sep-10 0:48
Andrey Permamedov24-Sep-10 0:48 
GeneralMy vote of 5 Pin
flyingxu21-Sep-10 19:23
flyingxu21-Sep-10 19:23 
GeneralRe: My vote of 5 Pin
Andrey Permamedov21-Sep-10 22:55
Andrey Permamedov21-Sep-10 22:55 
General[My vote of 2] There is many ways to solve this circs Pin
Carlos_never17-Sep-10 19:39
Carlos_never17-Sep-10 19:39 
GeneralRe: [My vote of 2] There is many ways to solve this circs Pin
Andrey Permamedov18-Sep-10 1:57
Andrey Permamedov18-Sep-10 1:57 
GeneralRe: [My vote of 2] There is many ways to solve this circs Pin
Carlos_never21-Sep-10 17:42
Carlos_never21-Sep-10 17:42 
GeneralRe: [My vote of 2] There is many ways to solve this circs Pin
Andrey Permamedov21-Sep-10 19:12
Andrey Permamedov21-Sep-10 19:12 
GeneralMy vote of 5 Pin
Blake Miller13-Sep-10 9:19
Blake Miller13-Sep-10 9:19 
GeneralRe: My vote of 5 Pin
Grump24-Jan-11 22:43
Grump24-Jan-11 22:43 

General General    News News    Suggestion Suggestion    Question Question    Bug Bug    Answer Answer    Joke Joke    Praise Praise    Rant Rant    Admin Admin   

Use Ctrl+Left/Right to switch messages, Ctrl+Up/Down to switch threads, Ctrl+Shift+Left/Right to switch pages.