Home Dashboard Directory Help
Search

MFC: bad hard typecast in CMFCToolBarMenuButton::CompareWith by tobias.loew


Status: 

Active


1
0
Sign in
to vote
Type: Bug
ID: 790246
Opened: 6/15/2013 8:01:48 AM
Access Restriction: Public
0
Workaround(s)
view
0
User(s) can reproduce this bug

Description

I got a crash that almost drove me nuts: the crash happened exclusively in release-builds and then only on instances not started immediately with a debugger. After several month a finally boiled the problem down to the line:

    const CMFCToolBarMenuButton& otherMenuBtn = (const CMFCToolBarMenuButton&) other;

in CMFCToolBarMenuButton::CompareWith.
One could argument that this is not a problem as just before compared the buttons are ensured to have the same ID, so "other" should be a CMFCToolBarMenuButton too.

But wait, this routine is also called during CMFCToolBar::SmartUpdate and I changed a toolbar-button from a former version to a toolbar-menu-button (while keeping the ID). Thus there is an illegal cast !!!

(In debug mode the line

    if (m_listCommands.GetCount() != otherMenuBtn.m_listCommands.GetCount())

usually return TRUE, so the crash does not occur in debug builds.)

So, please insert a DYNAMIC_DOWNCAST to check if "other" has the correct runtime type.

Thanks

Tobias
Details
Sign in to post a comment.
Posted by Microsoft on 4/29/2014 at 12:30 PM
Thank you for reporting this issue. This issue has been fixed in Visual Studio 2013. You can install a trial version of Visual Studio 2013 with the fix from: http://go.microsoft.com/?linkid=9832436
Posted by tobias.loew on 7/12/2013 at 2:05 AM
Great!
Is there any chance that older mfc-versions (version <= 11) will also get fixed? (At least mfc11 as VS2012 is still supported with updates.)

Tobias
Posted by Microsoft on 7/11/2013 at 2:31 PM
Hello,

Thanks for the report. This issue has been fixed in MFC for Visual Studio 2013 RTM.

Pat Brenner
Visual C++ Libraries Development
Posted by tobias.loew on 7/3/2013 at 1:18 AM
Hi,

two weeks have gone since MS's last comment to my post: Could you reproduce the error? Just for your info: apps mostly don't crash on this error, they silently execute illegal code and pass on but sometimes they crash (debug builds almost never crash due to explicite initialization of the padding bytes in allocations).

Tobias
Posted by tobias.loew on 6/19/2013 at 4:04 AM
Hi again,

I built a minimal project that can reproduze the error.
To reproduce:

1. Build the project
2. Run the app
2.1. Open "Customize"-Dialog, go to "Menu"-Tab, in "Select context menu"-ComboBox slect the "Edit" context menu
2.2. Close Custimize-Dialog and Exit App

(Point 2.1. ensures that in the Registry the keys
HKEY_CURRENT_USER\Software\Local AppWizard-Generated Applications\MFCApplication1\Workspace\BasePane-077
HKEY_CURRENT_USER\Software\Local AppWizard-Generated Applications\MFCApplication1\Workspace\MFCToolBar-077
HKEY_CURRENT_USER\Software\Local AppWizard-Generated Applications\MFCApplication1\Workspace\Pane-077
are created)

3. Reopen the App and you can reproduce the following stack

>    mfc110ud.dll!CMFCToolBarMenuButton::CompareWith(const CMFCToolBarButton & {...}) Line 1771    C++
    mfc110ud.dll!CMFCToolBar::SmartUpdate(const CObList & {...}) Line 6947    C++
    mfc110ud.dll!CMFCToolBar::LoadLastOriginalState(CSettingsStore & {...}) Line 7340    C++
    mfc110ud.dll!CMFCToolBar::LoadState(const wchar_t * 0x007f0dc8, int 0x00000000, unsigned int 0x00000077) Line 3788    C++
    mfc110ud.dll!CContextMenuManager::LoadState(const wchar_t * 0x007e5410) Line 418    C++
    mfc110ud.dll!CWinAppEx::LoadState(const wchar_t * 0x00000000, CFrameImpl * 0x007c1ab4) Line 436    C++
    mfc110ud.dll!CFrameImpl::OnLoadFrame() Line 215    C++
    mfc110ud.dll!CMDIFrameWndEx::LoadFrame(unsigned int 0x00000080, unsigned long 0x00cf8000, CWnd * 0x00000000, CCreateContext * 0x00000000) Line 433    C++
    MFCApplication1.exe!CMainFrame::LoadFrame(unsigned int 0x00000080, unsigned long 0x00cf8000, CWnd * 0x00000000, CCreateContext * 0x00000000) Line 308    C++
    MFCApplication1.exe!CMFCApplication1App::InitInstance() Line 125    C++
    mfc110ud.dll!AfxWinMain(HINSTANCE__ * 0x01240000, HINSTANCE__ * 0x00000000, wchar_t * 0x007a397c, int 0x0000000a) Line 37    C++
    MFCApplication1.exe!wWinMain(HINSTANCE__ * 0x01240000, HINSTANCE__ * 0x00000000, wchar_t * 0x007a397c, int 0x0000000a) Line 26    C++
    MFCApplication1.exe!__tmainCRTStartup() Line 528    C
    MFCApplication1.exe!wWinMainCRTStartup() Line 377    C
    kernel32.dll!<Unknown function>    Unknown
    ntdll.dll!<Unknown function>    Unknown
    ntdll.dll!<Unknown function>    Unknown


I uploaded the dump with heap to that stack.

I hope this ensures you, that the code is not only a serious bug, but could IMHO also be used for attacks.

regards

Tobias
Posted by Microsoft on 6/18/2013 at 12:37 AM
Thanks for your feedback.

We are rerouting this issue to the appropriate group within the Visual Studio Product Team for triage and resolution. These specialized experts will follow-up with your issue.
Posted by tobias.loew on 6/17/2013 at 4:10 AM
Hi,

I did the debugging of VS 2008, but control did not go to the second VS, instead I got the following call stack:




    ntdll.dll!_ZwRaiseException@12() + 0x12 Bytes    
    ntdll.dll!_ZwRaiseException@12() + 0x12 Bytes    
>    mfc90u.dll!CMFCToolBarMenuButton::CompareWith() + 0x45 Bytes    
    mfc90u.dll!CMFCToolBar::SmartUpdate() + 0x162 Bytes    
    mfc90u.dll!CMFCToolBar::LoadLastOriginalState() + 0x40 Bytes    
    mfc90u.dll!CMFCToolBar::LoadState() + 0x232 Bytes    
    mfc90u.dll!CContextMenuManager::LoadState() + 0x150 Bytes    
    mfc90u.dll!CWinAppEx::LoadState() + 0x2ae Bytes    
    ebs2000.exe!CEbs2000App::LoadState(const wchar_t * 0x00000000, CFrameImpl * 0x0ea0c5fc) Zeile 5969 + 0xc Bytes    C++
    mfc90u.dll!CFrameImpl::OnLoadFrame() + 0x2a Bytes    
    mfc90u.dll!CMDIFrameWndEx::LoadFrame() + 0x3c Bytes    
    ebs2000.exe!CMDIFrameEx::LoadFrame(unsigned int 152, unsigned long 13598720, CWnd * 0x00000000, CCreateContext * 0x00000000) Zeile 216 + 0x1e Bytes    C++
    ebs2000.exe!CEbs2000App::InitInstance() Zeile 2385 + 0x1c Bytes    C++
    mfc90u.dll!AfxWinMain() + 0x49 Bytes    
    ebs2000.exe!__tmainCRTStartup() Zeile 578 + 0x1c Bytes    C
    kernel32.dll!@BaseThreadInitThunk@12() + 0x12 Bytes    
    ntdll.dll!___RtlUserThreadStart@8() + 0x27 Bytes    
    ntdll.dll!__RtlUserThreadStart@8() + 0x1b Bytes    





---------------------------------------------------------------------------------------

After halting devenv in the second VS I generated the devenv-dump.
I also generated the dump for the crashed app and uploaded it also.

regards

Tobias
Posted by Microsoft on 6/17/2013 at 1:03 AM
Hi Tobias,

You can use the following steps to get a dump file:
1. Start Visual Studio.
2. Start another instance of VS.
3. In the second instance click Tools | Attach to Process...
4. In the list of processes locate devenv.exe.
5. Click OK and OK to close Select dialog and Attach to Process dialog.
6. Go back to the first instance of VS and repro the crash\hang.
7. Upon the crash\hang, control should go to the second instance of VS.
8. In the second instance click Debug | Save Dump (with heap).

If you are running the VB profile you will not see the Save Dump As menu item. To add this menu item:
a. Select Tools -> Customize
b. Select the Commands tab
c. Select Debug from the “Menu bar” dropdown
d. Click “Add Command…”
e. Select Debug from the Categories list.
f. Find the “Save Dump As” entry in the Commands window.
g. Click OK (the “Save Dump As…” command is added to the top of the Debug menu).
h. Click “Close”

Please zip the file and use "FeedbackID-790246" as prefix of the file name.
Posted by tobias.loew on 6/17/2013 at 12:51 AM
Hi,

I've uploaded the file as you requested.
I also did some tests and found out that the error is even more serious:

The serious problem is that CMFCToolBarMenuButtons (i.e. "sub-menu"-buttons) and CMFCToolBarButtons that are separators both have m_nID == 0. So, everytime a menu-button is compared with a separator CMFCToolBarMenuButton::CompareWith will be called and do the illegal cast and then anything can happen...
Posted by Microsoft on 6/16/2013 at 9:02 PM
Thanks for your feedback. In order to efficiently investigate and reproduce this issue, we are requesting additional information outlined below.
    
It may help if you provide us with Bucket ID, a dump file.

Bucket ID:
1.    Open Event Viewer        
2.    Click on Windows Logs -> Application
3.    Search for a 1001 event that occurred soon after you encountered the crash
4.    The bucket ID should be available in the event details
(there may be multiple 1001 events for any given crash - only one will have the bucket ID)
Dump File and Callstack:
You can get detailed steps about how to get the dump file and call stack at http://blogs.msdn.com/debugger/archive/2009/12/30/what-is-a-dump-and-how-do-i-create-one.aspx

************************************************************
Please zip the file and use "FeedbackID-790246" as prefix of the file name.

************************************************************
You can use the following workspace to upload the file:
https://sftus.one.microsoft.com/choosetransfer.aspx?key=5ec9b40d-a23d-4993-b38b-898cf2f200ce
Password is 7+vW$4Xo$x

Thanks again for your efforts and we look forward to hearing from you.

Microsoft Visual Studio Connect Support Team
Posted by Microsoft on 6/15/2013 at 8:51 AM
Thank you for your feedback, we are currently reviewing the issue you have submitted. If this issue is urgent, please contact support directly(http://support.microsoft.com)
Sign in to post a workaround.