MS07-012 Not Fixed

The MS07-012 patch that came out on Black Tuesday in Feb 2007 is not a
complete solution to the problem.

Title: MFC42u.dll Off-by-Two Overflow
Date: 15 March 2007
Affected: Windows 2000, XP, 2003 (those that were affected by the MS07-012
patch)
Reported by: Greg Sinclair (gssincla…nnlsoftware.com)

Overview:
The original MS07-012 patch was released to fix an issue in the MFC library
MFC42u.dll. The issue was the result of MS not taking into account that a
TCHAR string is actually twice as big as its CHAR counterparts. To fix this,
the patch readjusted the nMaxCount variable to half of its original value in
the GetMenuStringW(…) call. Unfortunately, GetMenuStringW will null
terminate a long string at the end adding two additional characters to the
string. This gives a returned string of (nMaxCount*2) + 2 bytes in size.

Details:
The original flaw was located in AfxOleSetEditMenu(). GetMenuString was
called with nMaxCount set to 0x200 for a buffer that was 0x200 bytes in
size. However, the buffer was TCHAR not char which lead to a buffer
overflow. According to Microsoft, the fixed source code now looks like:

    TCHAR szBuffer[256];
    pMenu->GetMenuString(iMenuItem, szBuffer, sizeof (szBuffer) / sizeof

(szBuffer[0]), MF_BYPOSITION);

If you look at the disassembly of this, you will see

lea eax, [ebp+szBuffer]
push 0x100 ; sizeof(szBuffer) / sizeof
(szBuffer[0]) == 0x100

This should have provided an exact fit for the returned string since
nMaxCount is equal to the size of szBuffer.

If you look at the MSDN description of GetMenuStringW (which is the function
actually called thanks to macros, and I suspect the root of the original
problem), you see that it clearly give the following warning:

"The nMaxCount parameter should be one larger than the number of characters
in the label to accommodate the null character that terminates a string."

The reason for this is that a string copy is called within GetMenuStringW
which will terminate a long string at nMaxCount. Since this function is
dealing with wide strings, this means an additional two 00 bytes will be put
into the buffer. Since the only local variable in AfxOleSetEditMenu() is
szBuffer, the stack frame looks like

szBuffer db 0x200
saved_ebp dd
saved_eip dd

This means that the last two bytes (LSW) of the original EBP will be
overwritten with NULL bytes. The calling function that calls
AfxOleSetEditMenu(…) will now have a corrupted EBP once AfxOleSetEditMenu
tears down its stack frame.

Exploiting this is by no means a trivial matter. But nevertheless, having
the ability to control another register in a system, especially a stack
frame, makes exploitation possible.

Workarounds:

• None.

Vendor Contact:
17 March 2007 - Attempted to report this to Microsoft four different times.
After I was told "I've documented your case. If enough people call about
this issue, we will see about fixing the patch" I decided to just release
this information. MS really should make it easier to report bugs!

greg.