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.

Attachment: smime.p7s
Description: S/MIME cryptographic signature



Relevant Pages

  • MS07-012 Not Fixed
    ... *The MS07-012 patch that came out on Black Tuesday in Feb 2007 is not a ... TCHAR string is actually twice as big as its CHAR counterparts. ... the patch readjusted the nMaxCount variable to half of its original value in ... 17 March 2007 - Attempted to report this to Microsoft four different times. ...
    (Vuln-Dev)
  • panic: tcp_log_addrs: string too long
    ... I have a tcp_log_addrs panic issue like following (with my patch). ... I didn't know why 'string too long'. ... to report this issue. ... KDB: stack backtrace: ...
    (freebsd-current)
  • Re: panic: tcp_log_addrs: string too long
    ... Andre Oppermann wrote: ... I have a tcp_log_addrs panic issue like following (with my patch). ... I didn't know why 'string too long'. ... to report this issue. ...
    (freebsd-current)
  • Re: SnapTheReport -- made it generic | example useage
    ... Sub SnapTheReport(pReportName As String, pFilter As String) ... ' pFilter = string to filter report or "" to get all ... Dim mFilename As String ... SetReportFilter pReportName, pFilter ...
    (microsoft.public.access.formscoding)
  • RE: Query form coding
    ... I have tried to include the query when using the wizard to design the report ... Dim strSource As String ... ' Remove Filter ...
    (microsoft.public.access.formscoding)